espressif/binutils-gdb
1
0
Fork 0
mirror of https://github.com/espressif/binutils-gdb.git synced 2025-06-06 07:28:44 +08:00
Code Issues Packages Projects Releases Wiki Activity

ld: pe: Apply review suggestions on the existing exports/imports arrays

Browse Source 
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.)
This commit is contained in:
Martin Storsjö
2022-09-06 18:39:07 +03:00
parent a33a94cf43
commit 825a844fdc
3 changed files with 83 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
ld/deffile.h
View File

@ -84,6 +84,7 @@ typedef struct def_file {
/* From the EXPORTS commands. */ /* From the EXPORTS commands. */
int num_exports; int num_exports;
unsigned int max_exports;
def_file_export *exports; def_file_export *exports;
/* Used by imports for module names. */ /* Used by imports for module names. */
@ -91,6 +92,7 @@ typedef struct def_file {
/* From the IMPORTS commands. */ /* From the IMPORTS commands. */
int num_imports; int num_imports;
unsigned int max_imports;
def_file_import *imports; def_file_import *imports;
/* From the VERSION command, -1 if not specified. */ /* From the VERSION command, -1 if not specified. */
@ -112,10 +114,10 @@ extern def_file *def_file_parse (const char *, def_file *);
extern void def_file_free (def_file *); extern void def_file_free (def_file *);
extern def_file_export *def_file_add_export (def_file *, const char *, extern def_file_export *def_file_add_export (def_file *, const char *,
const char *, int, const char *, int,
const char *, int *); const char *, bool *);
extern def_file_import *def_file_add_import (def_file *, const char *, extern def_file_import *def_file_add_import (def_file *, const char *,
const char *, int, const char *, const char *, int, const char *,
const char *, int *); const char *, bool *);
extern int def_file_add_import_from (def_file *fdef, extern int def_file_add_import_from (def_file *fdef,
int num_imports, int num_imports,
const char *name, const char *name,

127
ld/deffilep.y
View File

@ -459,8 +459,6 @@ def_file_free (def_file *fdef)
free (fdef->section_defs); 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) if (fdef->exports[i].internal_name != fdef->exports[i].name)
@ -469,10 +467,7 @@ def_file_free (def_file *fdef)
free (fdef->exports[i].its_name); free (fdef->exports[i].its_name);
} }
free (fdef->exports); 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) if (fdef->imports[i].internal_name != fdef->imports[i].name)
@ -481,7 +476,6 @@ def_file_free (def_file *fdef)
free (fdef->imports[i].its_name); free (fdef->imports[i].its_name);
} }
free (fdef->imports); free (fdef->imports);
}
while (fdef->modules) 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 /* Search the position of the identical element, or returns the position
of the next higher element. If last valid element is smaller, then MAX 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 static unsigned int
find_export_in_list (def_file_export *b, int max, find_export_in_list (def_file_export *b, unsigned int max,
const char *ex_name, const char *in_name, 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) if (!max)
return 0; return 0;
if ((e = cmp_export_elem (b, ex_name, in_name, its_name, ord)) <= 0) if ((e = cmp_export_elem (b, ex_name, in_name, its_name, ord)) <= 0)
{ {
if (!e) if (!e)
*is_ident = 1; *is_ident = true;
return 0; return 0;
} }
if (max == 1) if (max == 1)
@ -652,7 +649,7 @@ find_export_in_list (def_file_export *b, int max,
else if (!e || max == 2) else if (!e || max == 2)
{ {
if (!e) if (!e)
*is_ident = 1; *is_ident = true;
return max - 1; return max - 1;
} }
l = 0; r = 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); e = cmp_export_elem (b + p, ex_name, in_name, its_name, ord);
if (!e) if (!e)
{ {
*is_ident = 1; *is_ident = true;
return p; return p;
} }
else if (e < 0) 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) if ((e = cmp_export_elem (b + l, ex_name, in_name, its_name, ord)) > 0)
++l; ++l;
else if (!e) else if (!e)
*is_ident = 1; *is_ident = true;
return l; return l;
} }
@ -683,11 +680,10 @@ def_file_add_export (def_file *fdef,
const char *internal_name, const char *internal_name,
int ordinal, int ordinal,
const char *its_name, const char *its_name,
int *is_dup) bool *is_dup)
{ {
def_file_export *e; def_file_export *e;
int pos; unsigned int pos;
int max_exports = ROUND_UP(fdef->num_exports, 32);
if (internal_name && !external_name) if (internal_name && !external_name)
external_name = internal_name; external_name = internal_name;
@ -695,27 +691,27 @@ def_file_add_export (def_file *fdef,
internal_name = external_name; internal_name = external_name;
/* We need to avoid duplicates. */ /* We need to avoid duplicates. */
*is_dup = 0; *is_dup = false;
pos = find_export_in_list (fdef->exports, fdef->num_exports, pos = find_export_in_list (fdef->exports, fdef->num_exports,
external_name, internal_name, external_name, internal_name,
its_name, ordinal, is_dup); its_name, ordinal, is_dup);
if (*is_dup != 0) if (*is_dup)
return (fdef->exports + pos); 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); fdef->max_exports += SYMBOL_LIST_ARRAY_GROW;
if (fdef->exports)
fdef->exports = xrealloc (fdef->exports, fdef->exports = xrealloc (fdef->exports,
max_exports * sizeof (def_file_export)); fdef->max_exports * sizeof (def_file_export));
else
fdef->exports = xmalloc (max_exports * sizeof (def_file_export));
} }
e = fdef->exports + pos; 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))); 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)); memset (e, 0, sizeof (def_file_export));
e->name = xstrdup (external_name); e->name = xstrdup (external_name);
e->internal_name = xstrdup (internal_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 /* Search the position of the identical element, or returns the position
of the next higher element. If last valid element is smaller, then MAX 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 static unsigned int
find_import_in_list (def_file_import *b, int max, find_import_in_list (def_file_import *b, unsigned int max,
const char *ex_name, const char *in_name, 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) if (!max)
return 0; return 0;
if ((e = cmp_import_elem (b, ex_name, in_name, module, ord)) <= 0) if ((e = cmp_import_elem (b, ex_name, in_name, module, ord)) <= 0)
{ {
if (!e) if (!e)
*is_ident = 1; *is_ident = true;
return 0; return 0;
} }
if (max == 1) if (max == 1)
@ -797,7 +796,7 @@ find_import_in_list (def_file_import *b, int max,
else if (!e || max == 2) else if (!e || max == 2)
{ {
if (!e) if (!e)
*is_ident = 1; *is_ident = true;
return max - 1; return max - 1;
} }
l = 0; r = 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); e = cmp_import_elem (b + p, ex_name, in_name, module, ord);
if (!e) if (!e)
{ {
*is_ident = 1; *is_ident = true;
return p; return p;
} }
else if (e < 0) 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) if ((e = cmp_import_elem (b + l, ex_name, in_name, module, ord)) > 0)
++l; ++l;
else if (!e) else if (!e)
*is_ident = 1; *is_ident = true;
return l; return l;
} }
@ -849,33 +848,30 @@ def_file_add_import (def_file *fdef,
int ordinal, int ordinal,
const char *internal_name, const char *internal_name,
const char *its_name, const char *its_name,
int *is_dup) bool *is_dup)
{ {
def_file_import *i; def_file_import *i;
int pos; unsigned int pos;
int max_imports = ROUND_UP (fdef->num_imports, 16);
/* We need to avoid here duplicates. */ /* We need to avoid here duplicates. */
*is_dup = 0; *is_dup = false;
pos = find_import_in_list (fdef->imports, fdef->num_imports, pos = find_import_in_list (fdef->imports, fdef->num_imports,
name, name,
(!internal_name ? name : internal_name), (!internal_name ? name : internal_name),
module, ordinal, is_dup); module, ordinal, is_dup);
if (*is_dup != 0) if (*is_dup)
return fdef->imports + pos; 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); fdef->max_imports += SYMBOL_LIST_ARRAY_GROW;
if (fdef->imports)
fdef->imports = xrealloc (fdef->imports, fdef->imports = xrealloc (fdef->imports,
max_imports * sizeof (def_file_import)); fdef->max_imports * sizeof (def_file_import));
else
fdef->imports = xmalloc (max_imports * sizeof (def_file_import));
} }
i = fdef->imports + pos; 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)); memmove (i + 1, i, sizeof (def_file_import) * (fdef->num_imports - pos));
fill_in_import (i, name, def_stash_module (fdef, module), ordinal, 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) const char *its_name ATTRIBUTE_UNUSED)
{ {
def_file_import *i; def_file_import *i;
int is_dup; bool is_dup;
int pos; unsigned int pos;
int max_imports = ROUND_UP (fdef->num_imports, 16);
/* We need to avoid here duplicates. */ /* We need to avoid here duplicates. */
is_dup = 0; is_dup = false;
pos = find_import_in_list (fdef->imports, fdef->num_imports, pos = find_import_in_list (fdef->imports, fdef->num_imports,
name, internal_name ? internal_name : name, name, internal_name ? internal_name : name,
module, ordinal, &is_dup); module, ordinal, &is_dup);
if (is_dup != 0) if (is_dup)
return -1; return -1;
if (fdef->imports && pos != fdef->num_imports) if (fdef->imports && pos != (unsigned)fdef->num_imports)
{ {
i = fdef->imports + pos; i = fdef->imports + pos;
if (i->module && strcmp (i->module->name, module) == 0) if (i->module && strcmp (i->module->name, module) == 0)
return -1; 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, fdef->imports = xrealloc (fdef->imports,
max_imports * sizeof (def_file_import)); fdef->max_imports * sizeof (def_file_import));
else
fdef->imports = xmalloc (max_imports * sizeof (def_file_import));
} }
i = fdef->imports + pos; 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, memmove (i + num_imports, i,
sizeof (def_file_import) * (fdef->num_imports - pos)); sizeof (def_file_import) * (fdef->num_imports - pos));
@ -1261,7 +1256,7 @@ def_exports (const char *external_name,
const char *its_name) const char *its_name)
{ {
def_file_export *dfe; def_file_export *dfe;
int is_dup = 0; bool is_dup = false;
if (!internal_name && external_name) if (!internal_name && external_name)
internal_name = external_name; internal_name = external_name;
@ -1297,7 +1292,7 @@ def_import (const char *internal_name,
{ {
char *buf = 0; char *buf = 0;
const char *ext = dllext ? dllext : "dll"; const char *ext = dllext ? dllext : "dll";
int is_dup = 0; bool is_dup = false;
buf = xmalloc (strlen (module) + strlen (ext) + 2); buf = xmalloc (strlen (module) + strlen (ext) + 2);
sprintf (buf, "%s.%s", module, ext); sprintf (buf, "%s.%s", module, ext);

6
ld/pe-dll.c
View File

@ -791,7 +791,7 @@ process_def_file_and_drectve (bfd *abfd ATTRIBUTE_UNUSED, struct bfd_link_info *
if (auto_export (b, pe_def_file, sn)) if (auto_export (b, pe_def_file, sn))
{ {
int is_dup = 0; bool is_dup = false;
def_file_export *p; def_file_export *p;
p = def_file_add_export (pe_def_file, sn, 0, -1, p = def_file_add_export (pe_def_file, sn, 0, -1,
@ -857,7 +857,7 @@ process_def_file_and_drectve (bfd *abfd ATTRIBUTE_UNUSED, struct bfd_link_info *
if (strchr (pe_def_file->exports[i].name, '@')) if (strchr (pe_def_file->exports[i].name, '@'))
{ {
int is_dup = 1; bool is_dup = true;
int lead_at = (*pe_def_file->exports[i].name == '@'); int lead_at = (*pe_def_file->exports[i].name == '@');
char *tmp = xstrdup (pe_def_file->exports[i].name + lead_at); char *tmp = xstrdup (pe_def_file->exports[i].name + lead_at);
@ -3579,7 +3579,7 @@ pe_implied_import_dll (const char *filename)
exported in buggy auto-import releases. */ exported in buggy auto-import releases. */
if (! startswith (erva + name_rva, "__nm_")) if (! startswith (erva + name_rva, "__nm_"))
{ {
int is_dup = 0; bool is_dup = false;
/* is_data is true if the address is in the data, rdata or bss /* is_data is true if the address is in the data, rdata or bss
segment. */ segment. */
is_data = is_data =