Re: [PATCH 1/2] elflint: Pull pos() info file scope
Hi Timm, On Wed, 2021-02-17 at 09:42 +0100, Timm Bäder via Elfutils-devel wrote: > Rename it to buffer_pos() to be a bit more descriptive and get rid of a > nested function this way. Added a ChangeLog entry and pushed. Thanks, Mark
Re: [PATCH 2/2] elflint: Pull left() in file scope
Hi Timm, On Wed, 2021-02-17 at 09:42 +0100, Timm Bäder via Elfutils-devel wrote: > And rename it to buffer_left() to be a bit more descriptive Added a ChangeLog entry and pushed. Thanks, Mark
Re: [PATCH 1/5] unstrip: Pull adjust_reloc() into file scope
Hi Timm, On Wed, 2021-02-17 at 09:43 +0100, Timm Bäder via Elfutils-devel wrote: > Get rid of a nested function this way. Added a ChangeLog entry and pushed. Thanks, Mark
Re: [PATCH 2/5] unstrip: Pull check_match() into file scope
Hi Timm, On Wed, 2021-02-17 at 09:43 +0100, Timm Bäder via Elfutils-devel wrote: > Get rid of a nested function this way. Adding a ChangeLog entry explaining the new logic used and pushed. Thanks, Mark
Re: [PATCH 3/5] unstrip: Inline find_unalloc_section() into only caller
Hi Timm, On Wed, 2021-02-17 at 09:43 +0100, Timm Bäder via Elfutils-devel wrote: > Get rid of an unnecessary nested function this way. It took me a while to see why this was correct. Explained it in a new ChangeLog entry that I added before pushing. Thanks, Mark
Re: [PATCH 4/5] unstrip: Pull warn() into file scope
Hi Timm, On Wed, 2021-02-17 at 09:43 +0100, Timm Bäder via Elfutils-devel wrote: > Get rid of a nested function this way. Made one line smaller than 80 chars and added a ChangeLog entry. Pushed, Mark
Re: [PATCH 5/5] unstrip: Remove nested next() function
Hi Timm, On Wed, 2021-02-17 at 09:43 +0100, Timm Bäder via Elfutils-devel wrote: > This is a simple one-liner, so inline this into the few callers. > Get rid of a nested function this way. Added a ChangeLog entry and pushed. Thanks, 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/10/builds/693 Buildbot URL: https://builder.wildebeest.org/buildbot/ Worker for this Build: fedora-s390x Build Reason: Blamelist: Timm Bäder BUILD FAILED: failed test (failure) Sincerely, -The Buildbot
Re: [PATCH 1/4] elfcompress: Pull set_section() into file scope
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
Re: [PATCH 2/4] elfcompress: Pull get_section() into file scope
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
Re: [PATCH 3/4] elfcompress: Pull get_sections() into file scope
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
Re: [PATCH 4/4] elfcompress: Replace cleanup() with label
Hi Timm,
On Wed, 2021-02-17 at 09:45 +0100, Timm Bäder via Elfutils-devel wrote:
> From: Timm Bäder
>
> 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
Re: Buildbot failure in Wildebeest Builder on whole buildset
On Mon, 2021-03-01 at 19:56 +, build...@builder.wildebeest.org wrote: > 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/10/builds/693 > > Buildbot URL: https://builder.wildebeest.org/buildbot/ > > Worker for this Build: fedora-s390x > > Build Reason: > Blamelist: Timm Bäder > > BUILD FAILED: failed test (failure) I don't think it was Timm's patch. It seems the run-debuginfod-find.sh test is still unstable. But now it only happens on s390x. And there are at least 5 good builds between the failures. Cheers, Mark
Re: [PATCH 1/2] elfstrmerge: Pull newsecndx() info file scope
Hi Timm, On Wed, 2021-02-17 at 09:46 +0100, Timm Bäder via Elfutils-devel wrote: > Get rid of a nested function this way. Reworded the commit subject slightly to show this is about the tests and added a ChangeLog entry. Having to add 3 extra arguments to the function is a little ugly. But it is just a test. Pushed, Mark
Re: [PATCH 2/2] elfstrmerge: Pull new_data_buf() into file scope
Hi Timm, On Wed, 2021-02-17 at 09:46 +0100, Timm Bäder via Elfutils-devel wrote: > Get rid of a nested function this way. Same comments as the last one. 4 extra arguments is pushing it a bit. But still pushed :) Thanks, 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/10/builds/700 Buildbot URL: https://builder.wildebeest.org/buildbot/ Worker for this Build: fedora-s390x Build Reason: Blamelist: Timm Bäder BUILD FAILED: failed test (failure) Sincerely, -The Buildbot
