[PATCH] elfcompress: Replace cleanup() with label
From: Timm Bäder
This unifies the error handling with the rest of the code base and gets
rid of a nested function.
Signed-off-by: Timm Bäder
---
src/ChangeLog | 6 ++
src/elfcompress.c | 215 +++---
2 files changed, 112 insertions(+), 109 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 29f04e71..791015bb 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,9 @@
+2021-03-02 Timm Bäder
+
+ * elfcompress.c (process_file): Remove cleanup() function and
+ replace it with a cleanup label at the end of the function.
+ Initialize res to -1.
+
2021-02-17 Timm Bäder
* elfcompress.c (process_file): Move get_sections function...
diff --git a/src/elfcompress.c b/src/elfcompress.c
index 65e28f0e..c5ba6c34 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 = -1;
fd = open (fname, O_RDONLY);
if (fd < 0)
{
error (0, errno, "Couldn't open %s\n", fname);
- return cleanup (-1);
+ goto cleanup;
}
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 cleanup;
}
/* 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 cleanup;
}
struct stat st;
if (fstat (fd, &st) != 0)
{
error (0, errno, "Couldn't fstat %s", fname);
- return cleanup (-1);
+ goto cleanup;
}
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 cleanup;
}
/* 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 cleanup;
}
/* 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 cleanup;
}
if (shnum == 0)
{
error (0, 0, "ELF file %s has no sections", fname);
- return cleanup (-1);
+ goto cleanup;
}
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 cleanup;
}
/* 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 cleanup;
}
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 cleanup;
}
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 cleanup;
}
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 cleanup;
}
symtabndx = ndx;
}
@@ -558,7 +524,7 @@ process_file (const char *fname)
if (verbose > 0)
printf (
debuginfod: Remove always-false comparisons with LONG_MAX
If I understand the code correctly, these comparisons exist only for the curl_off_t cases, in which case dl and cl might be greater than LONG_MAX. However, in case they are declares as double values, this cannot be the case and the comparisons are unnecessary. - Timm
[PATCH] debuginfod-client: Remove always-false comparisons
From: Timm Bäder When comparing a long to a double, clang prints the following warning: ../../debuginfod/debuginfod-client.c:917:28: error: implicit conversion from 'long' to 'double' changes value from 9223372036854775807 to 9223372036854775808 [-Werror,-Wimplicit-int-float-conversion] pb = (cl > LONG_MAX ? LONG_MAX : (long)cl); ~ ^~~~ /usr/lib64/clang/10.0.1/include/limits.h:47:19: note: expanded from macro 'LONG_MAX' ^~~~ :38:22: note: expanded from here ^~~~ 1 error generated. However, cl and dl in are doubles and will never be greater than LONG_MAX, so always just assign them to pa or pb. Signed-off-by: Timm Bäder --- debuginfod/debuginfod-client.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c index de26af5b..c5f2ace5 100644 --- a/debuginfod/debuginfod-client.c +++ b/debuginfod/debuginfod-client.c @@ -896,7 +896,7 @@ debuginfod_query_server (debuginfod_client *c, CURLINFO_SIZE_DOWNLOAD, &dl); if (curl_res == 0) -pa = (dl > LONG_MAX ? LONG_MAX : (long)dl); +pa = (long)dl; #endif /* NB: If going through deflate-compressing proxies, this @@ -914,7 +914,7 @@ debuginfod_query_server (debuginfod_client *c, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &cl); if (curl_res == 0) -pb = (cl > LONG_MAX ? LONG_MAX : (long)cl); +pb = (long)cl; #endif } -- 2.26.2
Re: [PATCH] elfcompress: Replace cleanup() with label
Hi Timm, On Tue, 2021-03-02 at 09:05 +0100, Timm Bäder via Elfutils-devel wrote: > This unifies the error handling with the rest of the code base and > gets rid of a nested function. > > +2021-03-02 Timm Bäder > + > + * elfcompress.c (process_file): Remove cleanup() function and > + replace it with a cleanup label at the end of the function. > + Initialize res to -1. Thanks, pushed. Mark
Buildbot failure in Wildebeest Builder on whole buildset
The Buildbot has detected a failed build on builder whole buildset while building elfutils. Full details are available at: https://builder.wildebeest.org/buildbot/#builders/4/builds/733 Buildbot URL: https://builder.wildebeest.org/buildbot/ Worker for this Build: debian-i386 Build Reason: Blamelist: Timm Bäder BUILD FAILED: failed test (failure) Sincerely, -The Buildbot
Buildbot failure in Wildebeest Builder on whole buildset
The Buildbot has detected a failed build on builder whole buildset while building elfutils. Full details are available at: https://builder.wildebeest.org/buildbot/#builders/2/builds/734 Buildbot URL: https://builder.wildebeest.org/buildbot/ Worker for this Build: debian-amd64 Build Reason: Blamelist: Timm Bäder BUILD FAILED: failed test (failure) Sincerely, -The BuildbotThe Buildbot has detected a failed build on builder whole buildset while building elfutils. Full details are available at: https://builder.wildebeest.org/buildbot/#builders/3/builds/736 Buildbot URL: https://builder.wildebeest.org/buildbot/ Worker for this Build: fedora-x86_64 Build Reason: Blamelist: Timm Bäder BUILD FAILED: failed test (failure) Sincerely, -The Buildbot
Re: debuginfod: Remove always-false comparisons with LONG_MAX
Hi Timm, On Tue, 2021-03-02 at 09:30 +0100, Timm Bäder via Elfutils-devel wrote: > If I understand the code correctly, these comparisons exist only for > the > curl_off_t cases, in which case dl and cl might be greater than > LONG_MAX. However, in case they are declares as double values, this > cannot be the case and the comparisons are unnecessary. I asked on irc why we are doing this and it was suspected that it is becuase long on 32 bit can be smaller than the download size (returned in a double) So the suggestion was to change that to (cl > (double) LONG_MAX ? LONG_MAX : (long) cl); instead Cheers, Mark
[Bug debuginfod/27399] dpkg-deb/lzma error when indexing .debs
https://sourceware.org/bugzilla/show_bug.cgi?id=27399 Frank Ch. Eigler changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #8 from Frank Ch. Eigler --- Resolved under bug #27413 by deprecating use of dpkg-deb in favor of bsdtar. -- You are receiving this mail because: You are on the CC list for the bug.
Re: build-ids, .debug_sup and other IDs
>> But, this seemed a bit weird. What if both appear and they are >> different? Then a single API isn't so great -- you want to check the ID >> corresponding to whatever was in the original file. Frank> If both appear and are different, can we characterize the elf file as Frank> malformed? Not really, nothing specifies that these must be the same. Frank> Or debuginfod could export the content under -both- IDs, if there were Frank> two valid candidates, and just go with the flow. Let the clients Frank> choose which ID they prefer to look up by. There's a namespace problem here. You could, in theory, have executable A with build id , and also executable B with debug_sup id also . This could be fixed with some kind of query parameter. It would be easy on the gdb side to supply this information. Tom
Re: build-ids, .debug_sup and other IDs
Frank> (Does dwz'd dwarf5 even work on gdb Frank> etc. now?) It doesn't, this thread started because I sent a patch to change gdb to read .debug_sup. This hasn't landed yet. Tom
[Bug tools/27501] New: eu-readelf hang while process crafted file
https://sourceware.org/bugzilla/show_bug.cgi?id=27501 Bug ID: 27501 Summary: eu-readelf hang while process crafted file Product: elfutils Version: unspecified Status: UNCONFIRMED Severity: normal Priority: P2 Component: tools Assignee: unassigned at sourceware dot org Reporter: polaalemu at gmail dot com CC: elfutils-devel at sourceware dot org Target Milestone: --- Created attachment 13276 --> https://sourceware.org/bugzilla/attachment.cgi?id=13276&action=edit hang test case Hi, A hang was found in readelf v0.183 (the latest commit 35e49c) in function handle_symtab in readelf.c it seems verneed will always loop ,I didn't analysis deeply. bt [#0] 0x5556a2d3 → handle_symtab(shdr=0x7fffd600, scn=0x555dfa58, ebl=) [#1] 0x5556a2d3 → print_symtab(ebl=, type=0xb) [#2] 0x55575b95 → process_elf_file(dwflmod=0x555de300, fd=) [#3] 0x55578761 → process_dwflmod(dwflmod=0x555de300, userdata=, name=, base=, arg=0x7fffdb60) [#4] 0x77f7cc41 → dwfl_getmodules(dwfl=0x555de290, callback=0x55578730 , arg=0x7fffdb60, offset=0x0) [#5] 0x5556b811 → process_file(fd=0x3, fname=0x7fffe0a9 "../eu-readelf_hangs/id:00,src:002851,op:flip4,pos:5048", only_one=0x1) [#6] 0x555688de → main(argc=0x3, argv=0x7fffdce8) repruduce : eu-readelf -a [poc] -- You are receiving this mail because: You are on the CC list for the bug.
