* [binutils-gdb] ld: pe: Apply review suggestions on the existing exports/imports arrays
@ 2022-09-12 8:09 Martin Storsjo
0 siblings, 0 replies; only message in thread
From: Martin Storsjo @ 2022-09-12 8:09 UTC (permalink / raw)
To: bfd-cvs
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=825a844fdcfd4532cddde14d7bb38f1f981fa9fe
commit 825a844fdcfd4532cddde14d7bb38f1f981fa9fe
Author: Martin Storsjö <martin@martin.st>
Date: Tue Sep 6 18:39:07 2022 +0300
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.)
Diff:
---
ld/deffile.h | 6 ++-
ld/deffilep.y | 157 ++++++++++++++++++++++++++++------------------------------
ld/pe-dll.c | 6 +--
3 files changed, 83 insertions(+), 86 deletions(-)
diff --git a/ld/deffile.h b/ld/deffile.h
index 89ffe9f972e..9eb08ad83fb 100644
--- a/ld/deffile.h
+++ b/ld/deffile.h
@@ -84,6 +84,7 @@ typedef struct def_file {
/* From the EXPORTS commands. */
int num_exports;
+ unsigned int max_exports;
def_file_export *exports;
/* Used by imports for module names. */
@@ -91,6 +92,7 @@ typedef struct def_file {
/* From the IMPORTS commands. */
int num_imports;
+ unsigned int max_imports;
def_file_import *imports;
/* 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 def_file_export *def_file_add_export (def_file *, const char *,
const char *, int,
- const char *, int *);
+ const char *, bool *);
extern def_file_import *def_file_add_import (def_file *, const char *,
const char *, int, const char *,
- const char *, int *);
+ const char *, bool *);
extern int def_file_add_import_from (def_file *fdef,
int num_imports,
const char *name,
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);
diff --git a/ld/pe-dll.c b/ld/pe-dll.c
index 60584a88571..92c33f528c8 100644
--- a/ld/pe-dll.c
+++ b/ld/pe-dll.c
@@ -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))
{
- int is_dup = 0;
+ bool is_dup = false;
def_file_export *p;
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, '@'))
{
- int is_dup = 1;
+ bool is_dup = true;
int lead_at = (*pe_def_file->exports[i].name == '@');
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. */
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
segment. */
is_data =
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2022-09-12 8:09 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-12 8:09 [binutils-gdb] ld: pe: Apply review suggestions on the existing exports/imports arrays Martin Storsjo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).