Hi Timm,
On Wed, 2021-02-17 at 09:45 +0100, Timm Bäder via Elfutils-devel wrote:
> From: Timm Bäder <tbae...@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
