* [PATCH 1/4] elfcompress: Pull set_section() into file scope
@ 2021-02-17 8:45 tbaeder
2021-02-17 8:45 ` [PATCH 2/4] elfcompress: Pull get_section() " tbaeder
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: tbaeder @ 2021-02-17 8:45 UTC (permalink / raw)
To: elfutils-devel
From: Timm Bäder <tbaeder@redhat.com>
Get rid of a nested function this way.
Signed-off-by: Timm Bäder <tbaeder@redhat.com>
---
src/elfcompress.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/elfcompress.c b/src/elfcompress.c
index 1b5b1e36..65a922a7 100644
--- a/src/elfcompress.c
+++ b/src/elfcompress.c
@@ -52,6 +52,8 @@ static const char *foutput = NULL;
#define T_DECOMPRESS 1 /* none */
#define T_COMPRESS_ZLIB 2 /* zlib */
#define T_COMPRESS_GNU 3 /* zlib-gnu */
+#define WORD_BITS (8U * sizeof (unsigned int))
+
static int type = T_UNSET;
struct section_pattern
@@ -242,6 +244,12 @@ compress_section (Elf_Scn *scn, size_t orig_size, const char *name,
return res;
}
+static void
+set_section (unsigned int *sections, size_t ndx)
+{
+ sections[ndx / WORD_BITS] |= (1U << (ndx % WORD_BITS));
+}
+
static int
process_file (const char *fname)
{
@@ -275,12 +283,6 @@ process_file (const char *fname)
/* How many sections are we talking about? */
size_t shnum = 0;
-#define WORD_BITS (8U * sizeof (unsigned int))
- void set_section (size_t ndx)
- {
- sections[ndx / WORD_BITS] |= (1U << (ndx % WORD_BITS));
- }
-
bool get_section (size_t ndx)
{
return (sections[ndx / WORD_BITS] & (1U << (ndx % WORD_BITS))) != 0;
@@ -498,7 +500,7 @@ process_file (const char *fname)
else if (shdr->sh_type != SHT_NOBITS
&& (shdr->sh_flags & SHF_ALLOC) == 0)
{
- set_section (ndx);
+ set_section (sections, ndx);
/* Check if we might want to change this section name. */
if (! adjust_names
&& ((type != T_COMPRESS_GNU
--
2.26.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/4] elfcompress: Pull get_section() into file scope
2021-02-17 8:45 [PATCH 1/4] elfcompress: Pull set_section() into file scope tbaeder
@ 2021-02-17 8:45 ` tbaeder
2021-03-01 22:03 ` Mark Wielaard
2021-02-17 8:45 ` [PATCH 3/4] elfcompress: Pull get_sections() " tbaeder
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: tbaeder @ 2021-02-17 8:45 UTC (permalink / raw)
To: elfutils-devel
From: Timm Bäder <tbaeder@redhat.com>
Get rid of a nested function this way.
Signed-off-by: Timm Bäder <tbaeder@redhat.com>
---
src/elfcompress.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/elfcompress.c b/src/elfcompress.c
index 65a922a7..2dc74a0c 100644
--- a/src/elfcompress.c
+++ b/src/elfcompress.c
@@ -250,6 +250,12 @@ set_section (unsigned int *sections, size_t ndx)
sections[ndx / WORD_BITS] |= (1U << (ndx % WORD_BITS));
}
+static bool
+get_section (unsigned int *sections, size_t ndx)
+{
+ return (sections[ndx / WORD_BITS] & (1U << (ndx % WORD_BITS))) != 0;
+}
+
static int
process_file (const char *fname)
{
@@ -283,11 +289,6 @@ process_file (const char *fname)
/* How many sections are we talking about? */
size_t shnum = 0;
- bool get_section (size_t ndx)
- {
- return (sections[ndx / WORD_BITS] & (1U << (ndx % WORD_BITS))) != 0;
- }
-
/* How many sections are we going to change? */
size_t get_sections (void)
{
@@ -689,7 +690,7 @@ process_file (const char *fname)
/* (de)compress if section matched. */
char *sname = NULL;
char *newname = NULL;
- if (get_section (ndx))
+ if (get_section (sections, ndx))
{
GElf_Shdr shdr_mem;
GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_mem);
--
2.26.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/4] elfcompress: Pull get_sections() into file scope
2021-02-17 8:45 [PATCH 1/4] elfcompress: Pull set_section() into file scope tbaeder
2021-02-17 8:45 ` [PATCH 2/4] elfcompress: Pull get_section() " tbaeder
@ 2021-02-17 8:45 ` tbaeder
2021-03-01 22:04 ` Mark Wielaard
2021-02-17 8:45 ` [PATCH 4/4] elfcompress: Replace cleanup() with label tbaeder
2021-03-01 22:03 ` [PATCH 1/4] elfcompress: Pull set_section() into file scope Mark Wielaard
3 siblings, 1 reply; 8+ messages in thread
From: tbaeder @ 2021-02-17 8:45 UTC (permalink / raw)
To: elfutils-devel
From: Timm Bäder <tbaeder@redhat.com>
Get rid of a nested function this way.
Signed-off-by: Timm Bäder <tbaeder@redhat.com>
---
src/elfcompress.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/src/elfcompress.c b/src/elfcompress.c
index 2dc74a0c..65e28f0e 100644
--- a/src/elfcompress.c
+++ b/src/elfcompress.c
@@ -256,6 +256,16 @@ get_section (unsigned int *sections, size_t ndx)
return (sections[ndx / WORD_BITS] & (1U << (ndx % WORD_BITS))) != 0;
}
+/* How many sections are we going to change? */
+static size_t
+get_sections (unsigned int *sections, size_t shnum)
+{
+ size_t s = 0;
+ for (size_t i = 0; i < shnum / WORD_BITS + 1; i++)
+ s += __builtin_popcount (sections[i]);
+ return s;
+}
+
static int
process_file (const char *fname)
{
@@ -289,15 +299,6 @@ process_file (const char *fname)
/* How many sections are we talking about? */
size_t shnum = 0;
- /* How many sections are we going to change? */
- size_t get_sections (void)
- {
- size_t s = 0;
- for (size_t i = 0; i < shnum / WORD_BITS + 1; i++)
- s += __builtin_popcount (sections[i]);
- return s;
- }
-
int cleanup (int res)
{
elf_end (elf);
@@ -552,7 +553,7 @@ process_file (const char *fname)
}
}
- if (foutput == NULL && get_sections () == 0)
+ if (foutput == NULL && get_sections (sections, shnum) == 0)
{
if (verbose > 0)
printf ("Nothing to do.\n");
--
2.26.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 4/4] elfcompress: Replace cleanup() with label
2021-02-17 8:45 [PATCH 1/4] elfcompress: Pull set_section() into file scope tbaeder
2021-02-17 8:45 ` [PATCH 2/4] elfcompress: Pull get_section() " tbaeder
2021-02-17 8:45 ` [PATCH 3/4] elfcompress: Pull get_sections() " tbaeder
@ 2021-02-17 8:45 ` tbaeder
2021-03-01 22:10 ` Mark Wielaard
2021-03-01 22:03 ` [PATCH 1/4] elfcompress: Pull set_section() into file scope Mark Wielaard
3 siblings, 1 reply; 8+ messages in thread
From: tbaeder @ 2021-02-17 8:45 UTC (permalink / raw)
To: elfutils-devel
From: Timm Bäder <tbaeder@redhat.com>
Get rid of a nested function this way.
Signed-off-by: Timm Bäder <tbaeder@redhat.com>
---
src/elfcompress.c | 218 +++++++++++++++++++++++-----------------------
1 file changed, 109 insertions(+), 109 deletions(-)
diff --git a/src/elfcompress.c b/src/elfcompress.c
index 65e28f0e..ba08e73f 100644
--- a/src/elfcompress.c
+++ b/src/elfcompress.c
@@ -298,47 +298,13 @@ process_file (const char *fname)
/* How many sections are we talking about? */
size_t shnum = 0;
-
- int cleanup (int res)
- {
- elf_end (elf);
- close (fd);
-
- elf_end (elfnew);
- close (fdnew);
-
- if (fnew != NULL)
- {
- unlink (fnew);
- free (fnew);
- fnew = NULL;
- }
-
- free (snamebuf);
- if (names != NULL)
- {
- dwelf_strtab_free (names);
- free (scnstrents);
- free (symstrents);
- free (namesbuf);
- if (scnnames != NULL)
- {
- for (size_t n = 0; n < shnum; n++)
- free (scnnames[n]);
- free (scnnames);
- }
- }
-
- free (sections);
-
- return res;
- }
+ int res = 0;
fd = open (fname, O_RDONLY);
if (fd < 0)
{
error (0, errno, "Couldn't open %s\n", fname);
- return cleanup (-1);
+ goto error;
}
elf = elf_begin (fd, ELF_C_READ, NULL);
@@ -346,7 +312,7 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't open ELF file %s for reading: %s",
fname, elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
/* We don't handle ar files (or anything else), we probably should. */
@@ -357,21 +323,21 @@ process_file (const char *fname)
error (0, 0, "Cannot handle ar files: %s", fname);
else
error (0, 0, "Unknown file type: %s", fname);
- return cleanup (-1);
+ goto error;
}
struct stat st;
if (fstat (fd, &st) != 0)
{
error (0, errno, "Couldn't fstat %s", fname);
- return cleanup (-1);
+ goto error;
}
GElf_Ehdr ehdr;
if (gelf_getehdr (elf, &ehdr) == NULL)
{
error (0, 0, "Couldn't get ehdr for %s: %s", fname, elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
/* Get the section header string table. */
@@ -380,7 +346,7 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't get section header string table index in %s: %s",
fname, elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
/* How many sections are we talking about? */
@@ -388,13 +354,13 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't get number of sections in %s: %s",
fname, elf_errmsg (1));
- return cleanup (-1);
+ goto error;
}
if (shnum == 0)
{
error (0, 0, "ELF file %s has no sections", fname);
- return cleanup (-1);
+ goto error;
}
sections = xcalloc (shnum / 8 + 1, sizeof (unsigned int));
@@ -403,7 +369,7 @@ process_file (const char *fname)
if (elf_getphdrnum (elf, &phnum) != 0)
{
error (0, 0, "Couldn't get phdrnum: %s", elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
/* Whether we need to adjust any section names (going to/from GNU
@@ -460,7 +426,7 @@ process_file (const char *fname)
{
error (0, 0, "Unexpected section number %zd, expected only %zd",
ndx, shnum);
- cleanup (-1);
+ goto error;
}
GElf_Shdr shdr_mem;
@@ -468,14 +434,14 @@ process_file (const char *fname)
if (shdr == NULL)
{
error (0, 0, "Couldn't get shdr for section %zd", ndx);
- return cleanup (-1);
+ goto error;
}
const char *sname = elf_strptr (elf, shdrstrndx, shdr->sh_name);
if (sname == NULL)
{
error (0, 0, "Couldn't get name for section %zd", ndx);
- return cleanup (-1);
+ goto error;
}
if (section_name_matches (sname))
@@ -536,7 +502,7 @@ process_file (const char *fname)
{
error (0, 0,
"Multiple symbol tables (%zd, %zd) using the same string table unsupported", symtabndx, ndx);
- return cleanup (-1);
+ goto error;
}
symtabndx = ndx;
}
@@ -558,7 +524,7 @@ process_file (const char *fname)
if (verbose > 0)
printf ("Nothing to do.\n");
fnew = NULL;
- return cleanup (0);
+ goto cleanup;
}
if (adjust_names)
@@ -567,7 +533,7 @@ process_file (const char *fname)
if (names == NULL)
{
error (0, 0, "Not enough memory for new strtab");
- return cleanup (-1);
+ goto error;
}
scnstrents = xmalloc (shnum
* sizeof (Dwelf_Strent *));
@@ -594,7 +560,7 @@ process_file (const char *fname)
/* Since we didn't create it we don't want to try to unlink it. */
free (fnew);
fnew = NULL;
- return cleanup (-1);
+ goto error;
}
elfnew = elf_begin (fdnew, ELF_C_WRITE, NULL);
@@ -602,21 +568,21 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't open new ELF %s for writing: %s",
fnew, elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
/* Create the new ELF header and copy over all the data. */
if (gelf_newehdr (elfnew, gelf_getclass (elf)) == 0)
{
error (0, 0, "Couldn't create new ehdr: %s", elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
GElf_Ehdr newehdr;
if (gelf_getehdr (elfnew, &newehdr) == NULL)
{
error (0, 0, "Couldn't get new ehdr: %s", elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
newehdr.e_ident[EI_DATA] = ehdr.e_ident[EI_DATA];
@@ -629,7 +595,7 @@ process_file (const char *fname)
if (gelf_update_ehdr (elfnew, &newehdr) == 0)
{
error (0, 0, "Couldn't update ehdr: %s", elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
/* Copy over the phdrs as is. */
@@ -638,7 +604,7 @@ process_file (const char *fname)
if (gelf_newphdr (elfnew, phnum) == 0)
{
error (0, 0, "Couldn't create phdrs: %s", elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
for (size_t cnt = 0; cnt < phnum; ++cnt)
@@ -648,13 +614,13 @@ process_file (const char *fname)
if (phdr == NULL)
{
error (0, 0, "Couldn't get phdr %zd: %s", cnt, elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
if (gelf_update_phdr (elfnew, cnt, phdr) == 0)
{
error (0, 0, "Couldn't create phdr %zd: %s", cnt,
elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
}
}
@@ -698,7 +664,7 @@ process_file (const char *fname)
if (shdr == NULL)
{
error (0, 0, "Couldn't get shdr for section %zd", ndx);
- return cleanup (-1);
+ goto error;
}
uint64_t size = shdr->sh_size;
@@ -706,7 +672,7 @@ process_file (const char *fname)
if (sname == NULL)
{
error (0, 0, "Couldn't get name for section %zd", ndx);
- return cleanup (-1);
+ goto error;
}
/* strdup sname, the shdrstrndx section itself might be
@@ -728,7 +694,7 @@ process_file (const char *fname)
{
if (compress_section (scn, size, sname, NULL, ndx,
false, false, verbose > 0) < 0)
- return cleanup (-1);
+ goto error;
}
else if (strncmp (sname, ".zdebug", strlen (".zdebug")) == 0)
{
@@ -737,7 +703,7 @@ process_file (const char *fname)
newname = snamebuf;
if (compress_section (scn, size, sname, newname, ndx,
true, false, verbose > 0) < 0)
- return cleanup (-1);
+ goto error;
}
else if (verbose > 0)
printf ("[%zd] %s already decompressed\n", ndx, sname);
@@ -752,7 +718,7 @@ process_file (const char *fname)
Don't report even when verbose. */
if (compress_section (scn, size, sname, NULL, ndx,
false, false, false) < 0)
- return cleanup (-1);
+ goto error;
}
snamebuf[0] = '.';
@@ -779,13 +745,13 @@ process_file (const char *fname)
}
else
{
- int res = compress_section (scn, size, sname, newname,
- ndx, true, true,
- verbose > 0);
- if (res < 0)
- return cleanup (-1);
+ int result = compress_section (scn, size, sname, newname,
+ ndx, true, true,
+ verbose > 0);
+ if (result < 0)
+ goto error;
- if (res == 0)
+ if (result == 0)
newname = NULL;
}
}
@@ -809,7 +775,7 @@ process_file (const char *fname)
Don't report even when verbose. */
if (compress_section (scn, size, sname, NULL, ndx,
true, false, false) < 0)
- return cleanup (-1);
+ goto error;
snamebuf[0] = '.';
strcpy (&snamebuf[1], &sname[2]);
@@ -837,7 +803,7 @@ process_file (const char *fname)
}
else if (compress_section (scn, size, sname, newname, ndx,
false, true, verbose > 0) < 0)
- return cleanup (-1);
+ goto error;
}
else if (verbose > 0)
printf ("[%zd] %s already compressed\n", ndx, sname);
@@ -851,7 +817,7 @@ process_file (const char *fname)
if (newscn == NULL)
{
error (0, 0, "Couldn't create new section %zd", ndx);
- return cleanup (-1);
+ goto error;
}
GElf_Shdr shdr_mem;
@@ -859,13 +825,13 @@ process_file (const char *fname)
if (shdr == NULL)
{
error (0, 0, "Couldn't get shdr for section %zd", ndx);
- return cleanup (-1);
+ goto error;
}
if (gelf_update_shdr (newscn, shdr) == 0)
{
error (0, 0, "Couldn't update section header %zd", ndx);
- return cleanup (-1);
+ goto error;
}
/* Except for the section header string table all data can be
@@ -878,14 +844,14 @@ process_file (const char *fname)
if (data == NULL)
{
error (0, 0, "Couldn't get data from section %zd", ndx);
- return cleanup (-1);
+ goto error;
}
Elf_Data *newdata = elf_newdata (newscn);
if (newdata == NULL)
{
error (0, 0, "Couldn't create new data for section %zd", ndx);
- return cleanup (-1);
+ goto error;
}
*newdata = *data;
@@ -903,7 +869,7 @@ process_file (const char *fname)
if (name == NULL)
{
error (0, 0, "Couldn't get name for section [%zd]", ndx);
- return cleanup (-1);
+ goto error;
}
}
@@ -912,7 +878,7 @@ process_file (const char *fname)
if ((scnstrents[ndx] = dwelf_strtab_add (names, name)) == NULL)
{
error (0, 0, "No memory to add section name string table");
- return cleanup (-1);
+ goto error;
}
/* If the symtab shares strings then add those too. */
@@ -929,7 +895,7 @@ process_file (const char *fname)
/* Don't report the (internal) uncompression. */
if (compress_section (newscn, size, sname, NULL, ndx,
false, false, false) < 0)
- return cleanup (-1);
+ goto error;
symtab_size = size;
symtab_compressed = T_COMPRESS_ZLIB;
@@ -939,7 +905,7 @@ process_file (const char *fname)
/* Don't report the (internal) uncompression. */
if (compress_section (newscn, size, sname, NULL, ndx,
true, false, false) < 0)
- return cleanup (-1);
+ goto error;
symtab_size = size;
symtab_compressed = T_COMPRESS_GNU;
@@ -951,7 +917,7 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't get symtab data for section [%zd] %s",
ndx, name);
- return cleanup (-1);
+ goto error;
}
size_t elsize = gelf_fsize (elfnew, ELF_T_SYM, 1, EV_CURRENT);
size_t syms = symd->d_size / elsize;
@@ -963,7 +929,7 @@ process_file (const char *fname)
if (sym == NULL)
{
error (0, 0, "Couldn't get symbol %zd", i);
- return cleanup (-1);
+ goto error;
}
if (sym->st_name != 0)
{
@@ -975,13 +941,13 @@ process_file (const char *fname)
if (symname == NULL)
{
error (0, 0, "Couldn't get symbol %zd name", i);
- return cleanup (-1);
+ goto error;
}
symstrents[i] = dwelf_strtab_add (names, symname);
if (symstrents[i] == NULL)
{
error (0, 0, "No memory to add to symbol name");
- return cleanup (-1);
+ goto error;
}
}
}
@@ -1000,19 +966,19 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't get new section header string table [%zd]",
shdrstrndx);
- return cleanup (-1);
+ goto error;
}
Elf_Data *data = elf_newdata (scn);
if (data == NULL)
{
error (0, 0, "Couldn't create new section header string table data");
- return cleanup (-1);
+ goto error;
}
if (dwelf_strtab_finalize (names, data) == NULL)
{
error (0, 0, "Not enough memory to create string table");
- return cleanup (-1);
+ goto error;
}
namesbuf = data->d_buf;
@@ -1022,7 +988,7 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't get shdr for new section strings %zd",
shdrstrndx);
- return cleanup (-1);
+ goto error;
}
/* Note that we also might have to compress and possibly set
@@ -1042,7 +1008,7 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't update new section strings [%zd]",
shdrstrndx);
- return cleanup (-1);
+ goto error;
}
/* We might have to compress the data if the user asked us to,
@@ -1058,7 +1024,7 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't get section header string table [%zd]",
shdrstrndx);
- return cleanup (-1);
+ goto error;
}
shdr = gelf_getshdr (oldscn, &shdr_mem);
@@ -1066,7 +1032,7 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't get shdr for old section strings [%zd]",
shdrstrndx);
- return cleanup (-1);
+ goto error;
}
shstrtab_name = elf_strptr (elf, shdrstrndx, shdr->sh_name);
@@ -1074,7 +1040,7 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't get name for old section strings [%zd]",
shdrstrndx);
- return cleanup (-1);
+ goto error;
}
shstrtab_size = shdr->sh_size;
@@ -1091,7 +1057,7 @@ process_file (const char *fname)
shstrtab_newname, shdrstrndx,
shstrtab_compressed == T_COMPRESS_GNU,
true, verbose > 0) < 0)
- return cleanup (-1);
+ goto error;
}
}
@@ -1100,7 +1066,7 @@ process_file (const char *fname)
if (gelf_getehdr (elfnew, &newehdr) == NULL)
{
error (0, 0, "Couldn't re-get new ehdr: %s", elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
/* Set this after the sections have been created, otherwise section
@@ -1108,7 +1074,7 @@ process_file (const char *fname)
if (setshdrstrndx (elfnew, &newehdr, shdrstrndx) != 0)
{
error (0, 0, "Couldn't set new shdrstrndx: %s", elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
/* Fixup pass. Adjust string table references, symbol table and
@@ -1125,7 +1091,7 @@ process_file (const char *fname)
if (shdr == NULL)
{
error (0, 0, "Couldn't get shdr for section %zd", ndx);
- return cleanup (-1);
+ goto error;
}
/* Keep the offset of allocated sections so they are at the
@@ -1147,7 +1113,7 @@ process_file (const char *fname)
if (gelf_update_shdr (scn, shdr) == 0)
{
error (0, 0, "Couldn't update section header %zd", ndx);
- return cleanup (-1);
+ goto error;
}
if (adjust_names && ndx == symtabndx)
@@ -1160,7 +1126,7 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't get new symtab data section [%zd]",
ndx);
- return cleanup (-1);
+ goto error;
}
size_t elsize = gelf_fsize (elfnew, ELF_T_SYM, 1, EV_CURRENT);
size_t syms = symd->d_size / elsize;
@@ -1171,7 +1137,7 @@ process_file (const char *fname)
if (sym == NULL)
{
error (0, 0, "2 Couldn't get symbol %zd", i);
- return cleanup (-1);
+ goto error;
}
if (sym->st_name != 0)
@@ -1181,7 +1147,7 @@ process_file (const char *fname)
if (gelf_update_sym (symd, i, sym) == 0)
{
error (0, 0, "Couldn't update symbol %zd", i);
- return cleanup (-1);
+ goto error;
}
}
}
@@ -1199,7 +1165,7 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't get symbol table [%zd]",
symtabndx);
- return cleanup (-1);
+ goto error;
}
shdr = gelf_getshdr (oldscn, &shdr_mem);
@@ -1207,7 +1173,7 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't get old symbol table shdr [%zd]",
symtabndx);
- return cleanup (-1);
+ goto error;
}
symtab_name = elf_strptr (elf, shdrstrndx, shdr->sh_name);
@@ -1215,7 +1181,7 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't get old symbol table name [%zd]",
symtabndx);
- return cleanup (-1);
+ goto error;
}
symtab_size = shdr->sh_size;
@@ -1233,7 +1199,7 @@ process_file (const char *fname)
symtab_newname, symtabndx,
symtab_compressed == T_COMPRESS_GNU,
true, verbose > 0) < 0)
- return cleanup (-1);
+ goto error;
}
}
}
@@ -1247,7 +1213,7 @@ process_file (const char *fname)
if (gelf_getehdr (elfnew, &newehdr) == NULL)
{
error (0, 0, "Couldn't get ehdr: %s", elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
/* Position the shdrs after the last (unallocated) section. */
@@ -1262,7 +1228,7 @@ process_file (const char *fname)
if (gelf_update_ehdr (elfnew, &newehdr) == 0)
{
error (0, 0, "Couldn't update ehdr: %s", elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
}
@@ -1272,7 +1238,7 @@ process_file (const char *fname)
if (elf_update (elfnew, ELF_C_WRITE) < 0)
{
error (0, 0, "Couldn't write %s: %s", fnew, elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
elf_end (elfnew);
@@ -1294,14 +1260,48 @@ process_file (const char *fname)
if (rename (fnew, fname) != 0)
{
error (0, errno, "Couldn't rename %s to %s", fnew, fname);
- return cleanup (-1);
+ goto error;
}
/* We are finally done with the new file, don't unlink it now. */
free (fnew);
fnew = NULL;
+ goto cleanup;
+
+error:
+ res = -1;
+
+cleanup:
+ elf_end (elf);
+ close (fd);
- return cleanup (0);
+ elf_end (elfnew);
+ close (fdnew);
+
+ if (fnew != NULL)
+ {
+ unlink (fnew);
+ free (fnew);
+ fnew = NULL;
+ }
+
+ free (snamebuf);
+ if (names != NULL)
+ {
+ dwelf_strtab_free (names);
+ free (scnstrents);
+ free (symstrents);
+ free (namesbuf);
+ if (scnnames != NULL)
+ {
+ for (size_t n = 0; n < shnum; n++)
+ free (scnnames[n]);
+ free (scnnames);
+ }
+ }
+
+ free (sections);
+ return res;
}
int
--
2.26.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] elfcompress: Pull set_section() into file scope
2021-02-17 8:45 [PATCH 1/4] elfcompress: Pull set_section() into file scope tbaeder
` (2 preceding siblings ...)
2021-02-17 8:45 ` [PATCH 4/4] elfcompress: Replace cleanup() with label tbaeder
@ 2021-03-01 22:03 ` Mark Wielaard
3 siblings, 0 replies; 8+ messages in thread
From: Mark Wielaard @ 2021-03-01 22:03 UTC (permalink / raw)
To: tbaeder, elfutils-devel
Hi Timm,
On Wed, 2021-02-17 at 09:45 +0100, Timm Bäder via Elfutils-devel wrote:
> Get rid of a nested function this way.
Added a ChangeLog entry and pushed.
Thanks,
Mark
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/4] elfcompress: Pull get_section() into file scope
2021-02-17 8:45 ` [PATCH 2/4] elfcompress: Pull get_section() " tbaeder
@ 2021-03-01 22:03 ` Mark Wielaard
0 siblings, 0 replies; 8+ messages in thread
From: Mark Wielaard @ 2021-03-01 22:03 UTC (permalink / raw)
To: tbaeder, elfutils-devel
Hi Timm,
On Wed, 2021-02-17 at 09:45 +0100, Timm Bäder via Elfutils-devel wrote:
> Get rid of a nested function this way.
Added a ChangeLog entry and pushed.
Thanks,
Mark
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/4] elfcompress: Pull get_sections() into file scope
2021-02-17 8:45 ` [PATCH 3/4] elfcompress: Pull get_sections() " tbaeder
@ 2021-03-01 22:04 ` Mark Wielaard
0 siblings, 0 replies; 8+ messages in thread
From: Mark Wielaard @ 2021-03-01 22:04 UTC (permalink / raw)
To: tbaeder, elfutils-devel
Hi Timm,
On Wed, 2021-02-17 at 09:45 +0100, Timm Bäder via Elfutils-devel wrote:
> Get rid of a nested function this way.
Added a ChangeLog entry and pushed.
Thanks,
Mark
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 4/4] elfcompress: Replace cleanup() with label
2021-02-17 8:45 ` [PATCH 4/4] elfcompress: Replace cleanup() with label tbaeder
@ 2021-03-01 22:10 ` Mark Wielaard
0 siblings, 0 replies; 8+ messages in thread
From: Mark Wielaard @ 2021-03-01 22:10 UTC (permalink / raw)
To: tbaeder, elfutils-devel
Hi Timm,
On Wed, 2021-02-17 at 09:45 +0100, Timm Bäder via Elfutils-devel wrote:
> From: Timm Bäder <tbaeder@redhat.com>
>
> Get rid of a nested function this way.
It would be nice to get a ChangeLog entry with this.
> diff --git a/src/elfcompress.c b/src/elfcompress.c
> index 65e28f0e..ba08e73f 100644
> --- a/src/elfcompress.c
> +++ b/src/elfcompress.c
> @@ -298,47 +298,13 @@ process_file (const char *fname)
>
> /* How many sections are we talking about? */
> size_t shnum = 0;
> -
> - int cleanup (int res)
> - {
> - elf_end (elf);
> - close (fd);
> -
> - elf_end (elfnew);
> - close (fdnew);
> -
> - if (fnew != NULL)
> - {
> - unlink (fnew);
> - free (fnew);
> - fnew = NULL;
> - }
> -
> - free (snamebuf);
> - if (names != NULL)
> - {
> - dwelf_strtab_free (names);
> - free (scnstrents);
> - free (symstrents);
> - free (namesbuf);
> - if (scnnames != NULL)
> - {
> - for (size_t n = 0; n < shnum; n++)
> - free (scnnames[n]);
> - free (scnnames);
> - }
> - }
> -
> - free (sections);
> -
> - return res;
> - }
> + int res = 0;
If you are going to use a goto to the cleanup it makes sense to
initialize res to -1 here. Since till you get to the end, the result
will always be -1. And then you only need one (cleanup) label at the
end.
> @@ -1294,14 +1260,48 @@ process_file (const char *fname)
> if (rename (fnew, fname) != 0)
> {
> error (0, errno, "Couldn't rename %s to %s", fnew, fname);
> - return cleanup (-1);
> + goto error;
> }
>
> /* We are finally done with the new file, don't unlink it now. */
> free (fnew);
> fnew = NULL;
> + goto cleanup;
> +
> +error:
> + res = -1;
And then here instead of the goto cleanup and having an error label
simply set res = 0 and fall through to the cleanup label.
> +cleanup:
> + elf_end (elf);
> + close (fd);
>
> - return cleanup (0);
> + elf_end (elfnew);
> + close (fdnew);
> +
> + if (fnew != NULL)
> + {
> + unlink (fnew);
> + free (fnew);
> + fnew = NULL;
> + }
> +
> + free (snamebuf);
> + if (names != NULL)
> + {
> + dwelf_strtab_free (names);
> + free (scnstrents);
> + free (symstrents);
> + free (namesbuf);
> + if (scnnames != NULL)
> + {
> + for (size_t n = 0; n < shnum; n++)
> + free (scnnames[n]);
> + free (scnnames);
> + }
> + }
> +
> + free (sections);
> + return res;
> }
Cheers,
Mark
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-03-01 22:10 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-17 8:45 [PATCH 1/4] elfcompress: Pull set_section() into file scope tbaeder
2021-02-17 8:45 ` [PATCH 2/4] elfcompress: Pull get_section() " tbaeder
2021-03-01 22:03 ` Mark Wielaard
2021-02-17 8:45 ` [PATCH 3/4] elfcompress: Pull get_sections() " tbaeder
2021-03-01 22:04 ` Mark Wielaard
2021-02-17 8:45 ` [PATCH 4/4] elfcompress: Replace cleanup() with label tbaeder
2021-03-01 22:10 ` Mark Wielaard
2021-03-01 22:03 ` [PATCH 1/4] elfcompress: Pull set_section() into file scope Mark Wielaard
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).