diff options
author | Martin Storsjö <martin@martin.st> | 2022-09-06 18:39:07 +0300 |
---|---|---|
committer | Martin Storsjö <martin@martin.st> | 2022-09-12 11:07:35 +0300 |
commit | 825a844fdcfd4532cddde14d7bb38f1f981fa9fe (patch) | |
tree | 7a0e6ab9ef7c9bc1fbac6a8abbadef4c28453cce /ld/deffilep.y | |
parent | a33a94cf432e449461b7ddcc22b6f5a886cd3315 (diff) | |
download | binutils-gdb-825a844fdcfd4532cddde14d7bb38f1f981fa9fe.tar.gz |
ld: pe: Apply review suggestions on the existing exports/imports arrays
Use a separate explicit max_exports/imports field, instead of
deducing it from the number of allocated elements. Use a named
constant for the incremental growth of the array.
Use bool instead of int for boolean values.
Remove an unnecessary if statement/scope in the def_file_free
function.
Add more verbose comments about parameters, and about insertion
into an array of structs.
Generally use unsigned integers for all array indices and sizes.
The num_exports/imports fields are kept as is as signed integers,
since changing them to unsigned would require a disproportionate
amount of changes ti pe-dll.c to avoid comparisons between signed
and unsigned.
Simply use xrealloc instead of a check and xmalloc/xrealloc;
xrealloc can take NULL as the first parameter (and does a similar
check internally). (This wasn't requested in review though,
but noticed while working on the code.)
Diffstat (limited to 'ld/deffilep.y')
-rw-r--r-- | ld/deffilep.y | 157 |
1 files changed, 76 insertions, 81 deletions
diff --git a/ld/deffilep.y b/ld/deffilep.y index ed8f0d6719a..d7052a63beb 100644 --- a/ld/deffilep.y +++ b/ld/deffilep.y @@ -459,29 +459,23 @@ def_file_free (def_file *fdef) free (fdef->section_defs); } - if (fdef->exports) + for (i = 0; i < fdef->num_exports; i++) { - for (i = 0; i < fdef->num_exports; i++) - { - if (fdef->exports[i].internal_name != fdef->exports[i].name) - free (fdef->exports[i].internal_name); - free (fdef->exports[i].name); - free (fdef->exports[i].its_name); - } - free (fdef->exports); + if (fdef->exports[i].internal_name != fdef->exports[i].name) + free (fdef->exports[i].internal_name); + free (fdef->exports[i].name); + free (fdef->exports[i].its_name); } + free (fdef->exports); - if (fdef->imports) + for (i = 0; i < fdef->num_imports; i++) { - for (i = 0; i < fdef->num_imports; i++) - { - if (fdef->imports[i].internal_name != fdef->imports[i].name) - free (fdef->imports[i].internal_name); - free (fdef->imports[i].name); - free (fdef->imports[i].its_name); - } - free (fdef->imports); + if (fdef->imports[i].internal_name != fdef->imports[i].name) + free (fdef->imports[i].internal_name); + free (fdef->imports[i].name); + free (fdef->imports[i].its_name); } + free (fdef->imports); while (fdef->modules) { @@ -627,22 +621,25 @@ cmp_export_elem (const def_file_export *e, const char *ex_name, /* Search the position of the identical element, or returns the position of the next higher element. If last valid element is smaller, then MAX - is returned. */ + is returned. The max parameter indicates the number of elements in the + array. On return, *is_ident indicates whether the returned array index + points at an element which is identical to the one searched for. */ -static int -find_export_in_list (def_file_export *b, int max, +static unsigned int +find_export_in_list (def_file_export *b, unsigned int max, const char *ex_name, const char *in_name, - const char *its_name, int ord, int *is_ident) + const char *its_name, int ord, bool *is_ident) { - int e, l, r, p; + int e; + unsigned int l, r, p; - *is_ident = 0; + *is_ident = false; if (!max) return 0; if ((e = cmp_export_elem (b, ex_name, in_name, its_name, ord)) <= 0) { if (!e) - *is_ident = 1; + *is_ident = true; return 0; } if (max == 1) @@ -652,7 +649,7 @@ find_export_in_list (def_file_export *b, int max, else if (!e || max == 2) { if (!e) - *is_ident = 1; + *is_ident = true; return max - 1; } l = 0; r = max - 1; @@ -662,7 +659,7 @@ find_export_in_list (def_file_export *b, int max, e = cmp_export_elem (b + p, ex_name, in_name, its_name, ord); if (!e) { - *is_ident = 1; + *is_ident = true; return p; } else if (e < 0) @@ -673,7 +670,7 @@ find_export_in_list (def_file_export *b, int max, if ((e = cmp_export_elem (b + l, ex_name, in_name, its_name, ord)) > 0) ++l; else if (!e) - *is_ident = 1; + *is_ident = true; return l; } @@ -683,11 +680,10 @@ def_file_add_export (def_file *fdef, const char *internal_name, int ordinal, const char *its_name, - int *is_dup) + bool *is_dup) { def_file_export *e; - int pos; - int max_exports = ROUND_UP(fdef->num_exports, 32); + unsigned int pos; if (internal_name && !external_name) external_name = internal_name; @@ -695,27 +691,27 @@ def_file_add_export (def_file *fdef, internal_name = external_name; /* We need to avoid duplicates. */ - *is_dup = 0; + *is_dup = false; pos = find_export_in_list (fdef->exports, fdef->num_exports, external_name, internal_name, its_name, ordinal, is_dup); - if (*is_dup != 0) + if (*is_dup) return (fdef->exports + pos); - if (fdef->num_exports >= max_exports) + if ((unsigned)fdef->num_exports >= fdef->max_exports) { - max_exports = ROUND_UP(fdef->num_exports + 1, 32); - if (fdef->exports) - fdef->exports = xrealloc (fdef->exports, - max_exports * sizeof (def_file_export)); - else - fdef->exports = xmalloc (max_exports * sizeof (def_file_export)); + fdef->max_exports += SYMBOL_LIST_ARRAY_GROW; + fdef->exports = xrealloc (fdef->exports, + fdef->max_exports * sizeof (def_file_export)); } e = fdef->exports + pos; - if (pos != fdef->num_exports) + /* If we're inserting in the middle of the array, we need to move the + following elements forward. */ + if (pos != (unsigned)fdef->num_exports) memmove (&e[1], e, (sizeof (def_file_export) * (fdef->num_exports - pos))); + /* Wipe the element for use as a new entry. */ memset (e, 0, sizeof (def_file_export)); e->name = xstrdup (external_name); e->internal_name = xstrdup (internal_name); @@ -772,22 +768,25 @@ cmp_import_elem (const def_file_import *e, const char *ex_name, /* Search the position of the identical element, or returns the position of the next higher element. If last valid element is smaller, then MAX - is returned. */ + is returned. The max parameter indicates the number of elements in the + array. On return, *is_ident indicates whether the returned array index + points at an element which is identical to the one searched for. */ -static int -find_import_in_list (def_file_import *b, int max, +static unsigned int +find_import_in_list (def_file_import *b, unsigned int max, const char *ex_name, const char *in_name, - const char *module, int ord, int *is_ident) + const char *module, int ord, bool *is_ident) { - int e, l, r, p; + int e; + unsigned int l, r, p; - *is_ident = 0; + *is_ident = false; if (!max) return 0; if ((e = cmp_import_elem (b, ex_name, in_name, module, ord)) <= 0) { if (!e) - *is_ident = 1; + *is_ident = true; return 0; } if (max == 1) @@ -797,7 +796,7 @@ find_import_in_list (def_file_import *b, int max, else if (!e || max == 2) { if (!e) - *is_ident = 1; + *is_ident = true; return max - 1; } l = 0; r = max - 1; @@ -807,7 +806,7 @@ find_import_in_list (def_file_import *b, int max, e = cmp_import_elem (b + p, ex_name, in_name, module, ord); if (!e) { - *is_ident = 1; + *is_ident = true; return p; } else if (e < 0) @@ -818,7 +817,7 @@ find_import_in_list (def_file_import *b, int max, if ((e = cmp_import_elem (b + l, ex_name, in_name, module, ord)) > 0) ++l; else if (!e) - *is_ident = 1; + *is_ident = true; return l; } @@ -849,33 +848,30 @@ def_file_add_import (def_file *fdef, int ordinal, const char *internal_name, const char *its_name, - int *is_dup) + bool *is_dup) { def_file_import *i; - int pos; - int max_imports = ROUND_UP (fdef->num_imports, 16); + unsigned int pos; /* We need to avoid here duplicates. */ - *is_dup = 0; + *is_dup = false; pos = find_import_in_list (fdef->imports, fdef->num_imports, name, (!internal_name ? name : internal_name), module, ordinal, is_dup); - if (*is_dup != 0) + if (*is_dup) return fdef->imports + pos; - if (fdef->num_imports >= max_imports) + if ((unsigned)fdef->num_imports >= fdef->max_imports) { - max_imports = ROUND_UP (fdef->num_imports+1, 16); - - if (fdef->imports) - fdef->imports = xrealloc (fdef->imports, - max_imports * sizeof (def_file_import)); - else - fdef->imports = xmalloc (max_imports * sizeof (def_file_import)); + fdef->max_imports += SYMBOL_LIST_ARRAY_GROW; + fdef->imports = xrealloc (fdef->imports, + fdef->max_imports * sizeof (def_file_import)); } i = fdef->imports + pos; - if (pos != fdef->num_imports) + /* If we're inserting in the middle of the array, we need to move the + following elements forward. */ + if (pos != (unsigned)fdef->num_imports) memmove (i + 1, i, sizeof (def_file_import) * (fdef->num_imports - pos)); fill_in_import (i, name, def_stash_module (fdef, module), ordinal, @@ -895,36 +891,35 @@ def_file_add_import_from (def_file *fdef, const char *its_name ATTRIBUTE_UNUSED) { def_file_import *i; - int is_dup; - int pos; - int max_imports = ROUND_UP (fdef->num_imports, 16); + bool is_dup; + unsigned int pos; /* We need to avoid here duplicates. */ - is_dup = 0; + is_dup = false; pos = find_import_in_list (fdef->imports, fdef->num_imports, name, internal_name ? internal_name : name, module, ordinal, &is_dup); - if (is_dup != 0) + if (is_dup) return -1; - if (fdef->imports && pos != fdef->num_imports) + if (fdef->imports && pos != (unsigned)fdef->num_imports) { i = fdef->imports + pos; if (i->module && strcmp (i->module->name, module) == 0) return -1; } - if (fdef->num_imports + num_imports - 1 >= max_imports) + if ((unsigned)fdef->num_imports + num_imports - 1 >= fdef->max_imports) { - max_imports = ROUND_UP (fdef->num_imports + num_imports, 16); + fdef->max_imports = fdef->num_imports + num_imports + + SYMBOL_LIST_ARRAY_GROW; - if (fdef->imports) - fdef->imports = xrealloc (fdef->imports, - max_imports * sizeof (def_file_import)); - else - fdef->imports = xmalloc (max_imports * sizeof (def_file_import)); + fdef->imports = xrealloc (fdef->imports, + fdef->max_imports * sizeof (def_file_import)); } i = fdef->imports + pos; - if (pos != fdef->num_imports) + /* If we're inserting in the middle of the array, we need to move the + following elements forward. */ + if (pos != (unsigned)fdef->num_imports) memmove (i + num_imports, i, sizeof (def_file_import) * (fdef->num_imports - pos)); @@ -1261,7 +1256,7 @@ def_exports (const char *external_name, const char *its_name) { def_file_export *dfe; - int is_dup = 0; + bool is_dup = false; if (!internal_name && external_name) internal_name = external_name; @@ -1297,7 +1292,7 @@ def_import (const char *internal_name, { char *buf = 0; const char *ext = dllext ? dllext : "dll"; - int is_dup = 0; + bool is_dup = false; buf = xmalloc (strlen (module) + strlen (ext) + 2); sprintf (buf, "%s.%s", module, ext); |