Re: [PATCH 1/2] objcopy/strip: Allow section patterns starting with '!'.

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

* Alan Modra <amodra@gmail.com> [2016-07-14 18:19:24 +0930]:

> On Thu, Jul 14, 2016 at 09:26:47AM +0100, Andrew Burgess wrote:
> > diff --git a/binutils/NEWS b/binutils/NEWS
> > index 5bc6888..953d79f 100644
> > --- a/binutils/NEWS
> > +++ b/binutils/NEWS
> > @@ -8,6 +8,16 @@ Changes in 2.27:
> >  * Add --elf-stt-common= option to objcopy for ELF targets to control
> >    whether to convert common symbols to the STT_COMMON type.
> >  
> > +* The --remove-section option for objcopy and strip now accepts section
> > +  patterns starting with an exclamation point to indicate a non-matching
> > +  section.  A non-matching section is removed from the set of sections
> > +  matched by an earlier --remove-section pattern.
> > +
> > +* The --only-section option for objcopy now accepts section patterns
> > +  starting with an exclamation point to indicate a non-matching section.
> > +  A non-matching section is removed from the set of sections matched by
> > +  an earlier --only-section pattern.
> > +
> >  Changes in 2.26:
> >  
> >  * Add option to objcopy to insert new symbols into a file:
> 
> Needs to go above "Changes in 2.27".
> 
> > diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
> > index 5a004a3..dd60084 100644
> > --- a/binutils/doc/binutils.texi
> > +++ b/binutils/doc/binutils.texi
> > @@ -1221,6 +1221,18 @@
> >  inappropriately may make the output file unusable.  Wildcard
> >  characters are accepted in @var{sectionpattern}.
> >  
> > +If the first character of @var{sectionpattern} is the exclamation
> > +point (!) then matching sections will not be copied, even if earlier
> > +use of @option{--only-section} on the same command line would
> > +otherwise copy it.  Fo example:
> 
> Typo, repeated later too.
> 
> > diff --git a/binutils/objcopy.c b/binutils/objcopy.c
> > index 4bb625a..41ccc76 100644
> > --- a/binutils/objcopy.c
> > +++ b/binutils/objcopy.c
> > @@ -854,7 +854,7 @@ parse_symflags (const char *s, char **other)
> >  static struct section_list *
> >  find_section_list (const char *name, bfd_boolean add, unsigned int context)
> >  {
> > -  struct section_list *p;
> > +  struct section_list *p, *match = NULL;
> >  
> >    /* assert ((context & ((1 << 7) - 1)) != 0); */
> >  
> > @@ -890,19 +890,36 @@ find_section_list (const char *name, bfd_boolean add, unsigned int context)
> >  	}
> >        /* If we are not adding a new name/pattern then
> >  	 only check for a match if the context applies.  */
> > -      else if ((p->context & context)
> > -	       /* We could check for the presence of wildchar characters
> > -		  first and choose between calling strcmp and fnmatch,
> > -		  but is that really worth it ?  */
> > -	       && fnmatch (p->pattern, name, 0) == 0)
> > -	{
> > -	  p->used = TRUE;
> > -	  return p;
> > -	}
> > +      else if (p->context & context)
> 
> I suspect this will cause a gcc warning and thus build failure.
> Patch is OK once you've corrected these errors.

I'm a little surprised given that this is not actually new code, just
old code moved.  Not only that but there's plenty of similar example
of the same construct in the same function.

I tried compiling with -Wall -Werror on GCC 5.3.1, without warnings or
errors.  Are you expecting that the code should become:

      else if ((p->context & context) != 0)

I've fixed the other issues you pointed out in both patches - thanks
for the quick review.

Andrew