From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 7894) id 5C54B385AC35; Mon, 12 Sep 2022 08:09:56 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5C54B385AC35 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Martin Storsjo To: bfd-cvs@sourceware.org Subject: [binutils-gdb] ld: pe: Apply review suggestions on the existing exports/imports arrays X-Act-Checkin: binutils-gdb X-Git-Author: =?utf-8?q?Martin_Storsj=C3=B6?= X-Git-Refname: refs/heads/master X-Git-Oldrev: a33a94cf432e449461b7ddcc22b6f5a886cd3315 X-Git-Newrev: 825a844fdcfd4532cddde14d7bb38f1f981fa9fe Message-Id: <20220912080956.5C54B385AC35@sourceware.org> Date: Mon, 12 Sep 2022 08:09:56 +0000 (GMT) X-BeenThere: binutils-cvs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 12 Sep 2022 08:09:56 -0000 https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D825a844fdcfd= 4532cddde14d7bb38f1f981fa9fe commit 825a844fdcfd4532cddde14d7bb38f1f981fa9fe Author: Martin Storsj=C3=B6 Date: Tue Sep 6 18:39:07 2022 +0300 ld: pe: Apply review suggestions on the existing exports/imports arrays =20 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. =20 Use bool instead of int for boolean values. =20 Remove an unnecessary if statement/scope in the def_file_free function. =20 Add more verbose comments about parameters, and about insertion into an array of structs. =20 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. =20 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 { =20 /* From the EXPORTS commands. */ int num_exports; + unsigned int max_exports; def_file_export *exports; =20 /* Used by imports for module names. */ @@ -91,6 +92,7 @@ typedef struct def_file { =20 /* From the IMPORTS commands. */ int num_imports; + unsigned int max_imports; def_file_import *imports; =20 /* From the VERSION command, -1 if not specified. */ @@ -112,10 +114,10 @@ extern def_file *def_file_parse (const char *, def_fi= le *); 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); } =20 - if (fdef->exports) + for (i =3D 0; i < fdef->num_exports; i++) { - for (i =3D 0; i < fdef->num_exports; i++) - { - if (fdef->exports[i].internal_name !=3D 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 !=3D fdef->exports[i].name) + free (fdef->exports[i].internal_name); + free (fdef->exports[i].name); + free (fdef->exports[i].its_name); } + free (fdef->exports); =20 - if (fdef->imports) + for (i =3D 0; i < fdef->num_imports; i++) { - for (i =3D 0; i < fdef->num_imports; i++) - { - if (fdef->imports[i].internal_name !=3D 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 !=3D fdef->imports[i].name) + free (fdef->imports[i].internal_name); + free (fdef->imports[i].name); + free (fdef->imports[i].its_name); } + free (fdef->imports); =20 while (fdef->modules) { @@ -627,22 +621,25 @@ cmp_export_elem (const def_file_export *e, const char= *ex_name, =20 /* 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. */ =20 -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; =20 - *is_ident =3D 0; + *is_ident =3D false; if (!max) return 0; if ((e =3D cmp_export_elem (b, ex_name, in_name, its_name, ord)) <=3D 0) { if (!e) - *is_ident =3D 1; + *is_ident =3D true; return 0; } if (max =3D=3D 1) @@ -652,7 +649,7 @@ find_export_in_list (def_file_export *b, int max, else if (!e || max =3D=3D 2) { if (!e) - *is_ident =3D 1; + *is_ident =3D true; return max - 1; } l =3D 0; r =3D max - 1; @@ -662,7 +659,7 @@ find_export_in_list (def_file_export *b, int max, e =3D cmp_export_elem (b + p, ex_name, in_name, its_name, ord); if (!e) { - *is_ident =3D 1; + *is_ident =3D true; return p; } else if (e < 0) @@ -673,7 +670,7 @@ find_export_in_list (def_file_export *b, int max, if ((e =3D cmp_export_elem (b + l, ex_name, in_name, its_name, ord)) > 0) ++l; else if (!e) - *is_ident =3D 1; + *is_ident =3D true; return l; } =20 @@ -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 =3D ROUND_UP(fdef->num_exports, 32); + unsigned int pos; =20 if (internal_name && !external_name) external_name =3D internal_name; @@ -695,27 +691,27 @@ def_file_add_export (def_file *fdef, internal_name =3D external_name; =20 /* We need to avoid duplicates. */ - *is_dup =3D 0; + *is_dup =3D false; pos =3D find_export_in_list (fdef->exports, fdef->num_exports, external_name, internal_name, its_name, ordinal, is_dup); =20 - if (*is_dup !=3D 0) + if (*is_dup) return (fdef->exports + pos); =20 - if (fdef->num_exports >=3D max_exports) + if ((unsigned)fdef->num_exports >=3D fdef->max_exports) { - max_exports =3D ROUND_UP(fdef->num_exports + 1, 32); - if (fdef->exports) - fdef->exports =3D xrealloc (fdef->exports, - max_exports * sizeof (def_file_export)); - else - fdef->exports =3D xmalloc (max_exports * sizeof (def_file_export)); + fdef->max_exports +=3D SYMBOL_LIST_ARRAY_GROW; + fdef->exports =3D xrealloc (fdef->exports, + fdef->max_exports * sizeof (def_file_export)); } =20 e =3D fdef->exports + pos; - if (pos !=3D fdef->num_exports) + /* If we're inserting in the middle of the array, we need to move the + following elements forward. */ + if (pos !=3D (unsigned)fdef->num_exports) memmove (&e[1], e, (sizeof (def_file_export) * (fdef->num_exports - po= s))); + /* Wipe the element for use as a new entry. */ memset (e, 0, sizeof (def_file_export)); e->name =3D xstrdup (external_name); e->internal_name =3D xstrdup (internal_name); @@ -772,22 +768,25 @@ cmp_import_elem (const def_file_import *e, const char= *ex_name, =20 /* 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. */ =20 -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; =20 - *is_ident =3D 0; + *is_ident =3D false; if (!max) return 0; if ((e =3D cmp_import_elem (b, ex_name, in_name, module, ord)) <=3D 0) { if (!e) - *is_ident =3D 1; + *is_ident =3D true; return 0; } if (max =3D=3D 1) @@ -797,7 +796,7 @@ find_import_in_list (def_file_import *b, int max, else if (!e || max =3D=3D 2) { if (!e) - *is_ident =3D 1; + *is_ident =3D true; return max - 1; } l =3D 0; r =3D max - 1; @@ -807,7 +806,7 @@ find_import_in_list (def_file_import *b, int max, e =3D cmp_import_elem (b + p, ex_name, in_name, module, ord); if (!e) { - *is_ident =3D 1; + *is_ident =3D true; return p; } else if (e < 0) @@ -818,7 +817,7 @@ find_import_in_list (def_file_import *b, int max, if ((e =3D cmp_import_elem (b + l, ex_name, in_name, module, ord)) > 0) ++l; else if (!e) - *is_ident =3D 1; + *is_ident =3D true; return l; } =20 @@ -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 =3D ROUND_UP (fdef->num_imports, 16); + unsigned int pos; =20 /* We need to avoid here duplicates. */ - *is_dup =3D 0; + *is_dup =3D false; pos =3D find_import_in_list (fdef->imports, fdef->num_imports, name, (!internal_name ? name : internal_name), module, ordinal, is_dup); - if (*is_dup !=3D 0) + if (*is_dup) return fdef->imports + pos; =20 - if (fdef->num_imports >=3D max_imports) + if ((unsigned)fdef->num_imports >=3D fdef->max_imports) { - max_imports =3D ROUND_UP (fdef->num_imports+1, 16); - - if (fdef->imports) - fdef->imports =3D xrealloc (fdef->imports, - max_imports * sizeof (def_file_import)); - else - fdef->imports =3D xmalloc (max_imports * sizeof (def_file_import)); + fdef->max_imports +=3D SYMBOL_LIST_ARRAY_GROW; + fdef->imports =3D xrealloc (fdef->imports, + fdef->max_imports * sizeof (def_file_import)); } i =3D fdef->imports + pos; - if (pos !=3D fdef->num_imports) + /* If we're inserting in the middle of the array, we need to move the + following elements forward. */ + if (pos !=3D (unsigned)fdef->num_imports) memmove (i + 1, i, sizeof (def_file_import) * (fdef->num_imports - pos= )); =20 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 =3D ROUND_UP (fdef->num_imports, 16); + bool is_dup; + unsigned int pos; =20 /* We need to avoid here duplicates. */ - is_dup =3D 0; + is_dup =3D false; pos =3D find_import_in_list (fdef->imports, fdef->num_imports, name, internal_name ? internal_name : name, module, ordinal, &is_dup); - if (is_dup !=3D 0) + if (is_dup) return -1; - if (fdef->imports && pos !=3D fdef->num_imports) + if (fdef->imports && pos !=3D (unsigned)fdef->num_imports) { i =3D fdef->imports + pos; if (i->module && strcmp (i->module->name, module) =3D=3D 0) return -1; } =20 - if (fdef->num_imports + num_imports - 1 >=3D max_imports) + if ((unsigned)fdef->num_imports + num_imports - 1 >=3D fdef->max_imports) { - max_imports =3D ROUND_UP (fdef->num_imports + num_imports, 16); + fdef->max_imports =3D fdef->num_imports + num_imports + + SYMBOL_LIST_ARRAY_GROW; =20 - if (fdef->imports) - fdef->imports =3D xrealloc (fdef->imports, - max_imports * sizeof (def_file_import)); - else - fdef->imports =3D xmalloc (max_imports * sizeof (def_file_import)); + fdef->imports =3D xrealloc (fdef->imports, + fdef->max_imports * sizeof (def_file_import)); } i =3D fdef->imports + pos; - if (pos !=3D fdef->num_imports) + /* If we're inserting in the middle of the array, we need to move the + following elements forward. */ + if (pos !=3D (unsigned)fdef->num_imports) memmove (i + num_imports, i, sizeof (def_file_import) * (fdef->num_imports - pos)); =20 @@ -1261,7 +1256,7 @@ def_exports (const char *external_name, const char *its_name) { def_file_export *dfe; - int is_dup =3D 0; + bool is_dup =3D false; =20 if (!internal_name && external_name) internal_name =3D external_name; @@ -1297,7 +1292,7 @@ def_import (const char *internal_name, { char *buf =3D 0; const char *ext =3D dllext ? dllext : "dll"; - int is_dup =3D 0; + bool is_dup =3D false; =20 buf =3D 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_UNUSE= D, struct bfd_link_info * =20 if (auto_export (b, pe_def_file, sn)) { - int is_dup =3D 0; + bool is_dup =3D false; def_file_export *p; =20 p =3D def_file_add_export (pe_def_file, sn, 0, -1, @@ -857,7 +857,7 @@ process_def_file_and_drectve (bfd *abfd ATTRIBUTE_UNUSE= D, struct bfd_link_info * =20 if (strchr (pe_def_file->exports[i].name, '@')) { - int is_dup =3D 1; + bool is_dup =3D true; int lead_at =3D (*pe_def_file->exports[i].name =3D=3D '@'); char *tmp =3D xstrdup (pe_def_file->exports[i].name + lead_at); =20 @@ -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 =3D 0; + bool is_dup =3D false; /* is_data is true if the address is in the data, rdata or bss segment. */ is_data =3D