[PATCH 2/2] Improve reporting of section conflict errors

[Richard Guenther]

On Thu, Dec 16, 2010 at 2:31 PM, Andi Kleen <andi@firstfloor.org> wrote:
> On Thu, Dec 16, 2010 at 02:25:05PM +0100, Richard Guenther wrote:
>> > +  struct flag *fl;
>> > +  char *start = buf;
>> > +
>> > +  f &= ~SECTION_OVERRIDE;
>>
>> Any reason to do this here and not in the caller?
>
> I can move it to the caller.

Thanks.

>>
>> Can you split out a helper function that does return a reference to
>> the name for a single section flag?  I suppose it might be useful
>> on its own.
>
> What do you mean with single? If you pass only a single flag you should
> already know what it is? I think this is general enough for most
> debugging purposes.

Single as in expose the local array you have via a helper function.
But well, if you think that's not useful then leave it to whoever would
have a use for it.

>> > @@ -290,15 +343,23 @@ get_section (const char *name, unsigned int flags, tree decl)
>> >     }
>> >   else
>> >     {
>> > +      unsigned oflags;
>> > +
>> >       sect = *slot;
>> > -      if ((sect->common.flags & ~SECTION_DECLARED) != flags
>> > +      oflags = sect->common.flags & ~SECTION_DECLARED;
>> > +      if (oflags != flags
>> >          && ((sect->common.flags | flags) & SECTION_OVERRIDE) == 0)
>> >        {
>> > +         char buf[1024];
>>
>> Ick ... but well.  I suppose you could use asprintf in section_print_flags
>> (which is oddly named, better would be section_flags_string).  The function
>> shouldn't be timing critical.
>
> I can rename it.

Thanks.  Please also make the array const as suggested.

>>
>> I agree the patch is useful.
>
> Is that an approval?

I'd like to see an updated patch with the fixed size array removed.

Richard.

> -Andi
>
> --
> ak@linux.intel.com -- Speaking for myself only.
>