Re: [PATCH 1/2] objcopy: Factor out some of the option parsing code.

[Andrew Burgess <andrew dot burgess at embecosm dot com>]

* Steve Ellcey <sellcey@imgtec.com> [2015-02-26 08:51:41 -0800]:

> On Mon, 2015-02-23 at 12:52 +0000, Andrew Burgess wrote:
> > This patch splits out some of the option parsing code, increasing code
> > reuse.
> > 
> > binutils/ChangeLog:
> > 
> > 	* objcopy.c (init_section_add): New function.
> > 	(section_add_load_file): New function.
> > 	(copy_main): Make use of new functions.
> 
> > +static struct section_add *
> > +init_section_add (const char *optarg,
> > +                  struct section_add *next,
> > +                  const char *option)
>
> This change is breaking the binutils build for me.  The build dies with:
> 
> /scratch/sellcey/repos/objcopy/src/binutils/binutils/objcopy.c: In function 'init_section_add':
> /scratch/sellcey/repos/objcopy/src/binutils/binutils/objcopy.c:3556:31: error: declaration of 'optarg' shadows a global declaration [-Werror=shadow]

That error looks correct.  Strangely though using:
  gcc (GCC) 4.8.3 20140911 (Red Hat 4.8.3-7)
I'm not seeing the error message, even though I do have optarg
available as a global .... very strange.

Does the patch below fix the issue for you?  I rename optarg to arg,
which I don't believe is a global, but given that I have no faith in
the shadow detection in my version of GCC, I'm nervous to just push
this without it being confirmed to fix the issue.

If you can confirm this then I'll push it.  Alternatively, I'll setup
an alternative GCC to compile with that hopefully will give me the
shadow warning you see.

Sorry for the breakage,
Andrew

---

In commit 7173b38a442c007a554ea200817a0eadce89c87b I used optarg as the
name for a function parameter, shadowing the global of that name.  This
commit changes the function parameter to be called arg.

binutils/ChangeLog:

	* objcopy.c (init_section_add): Rename optarg to arg in order to
	avoid shadowing a global variable.
---
 binutils/ChangeLog | 5 +++++
 binutils/objcopy.c | 8 ++++----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 3a42b72..6caa4fa 100644
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,8 @@
+2015-02-26  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* objcopy.c (init_section_add): Rename optarg to arg in order to
+	avoid shadowing a global variable.
+
 2015-02-26  Nick Clifton  <nickc@redhat.com>
 
 	PR binutils/17512
diff --git a/binutils/objcopy.c b/binutils/objcopy.c
index 5cb4b13..7f094d3 100644
--- a/binutils/objcopy.c
+++ b/binutils/objcopy.c
@@ -3547,25 +3547,25 @@ convert_efi_target (char *efi)
 }
 
 /* Allocate and return a pointer to a struct section_add, initializing the
-   structure using OPTARG, a string in the format "sectionname=filename".
+   structure using ARG, a string in the format "sectionname=filename".
    The returned structure will have its next pointer set to NEXT.  The
    OPTION field is the name of the command line option currently being
    parsed, and is only used if an error needs to be reported.  */
 
 static struct section_add *
-init_section_add (const char *optarg,
+init_section_add (const char *arg,
                   struct section_add *next,
                   const char *option)
 {
   struct section_add *pa;
   const char *s;
 
-  s = strchr (optarg, '=');
+  s = strchr (arg, '=');
   if (s == NULL)
     fatal (_("bad format for %s"), option);
 
   pa = (struct section_add *) xmalloc (sizeof (struct section_add));
-  pa->name = xstrndup (optarg, s - optarg);
+  pa->name = xstrndup (arg, s - arg);
   pa->filename = s + 1;
   pa->next = next;
   pa->contents = NULL;
-- 
2.2.2