Re: [PATCH] gettext: Fix overloadable error with clang

[Adrian Bunk <bunk@stusta.de>]

On Wed, Jan 22, 2020 at 12:28:02PM -0800, Khem Raj wrote:
> On Tue, Jan 21, 2020 at 8:32 AM Adrian Bunk <bunk@stusta.de> wrote:
> >
> > On Thu, Jan 16, 2020 at 07:17:20AM -0800, Khem Raj wrote:
> > > On Thu, Jan 16, 2020 at 5:13 AM Adrian Bunk <bunk@stusta.de> wrote:
> > > >
> > > > On Wed, Jan 15, 2020 at 08:46:09PM -0800, Khem Raj wrote:
> > > > > Clang detects that getcwd is being re-declared and signatures don't
> > > > > match, simple solution is to let clang use overloadable attribute
> > > > >...
> > > > > +Fixes
> > > > > +dcigettext.c:147:7: error: redeclaration of 'getcwd' must have the 'overloadable' attribute
> > > > >...
> > > > > +-char *getcwd ();
> > > > >...
> > > >
> > > > Looks like a bug in clang to me, and should be fixed there.
> > > >
> > > > The code does not tell anything regarding the parameters,
> > > > but clang seems to misinterpret it as "no parameters".
> > > >
> > > its conflicting with declaration from glibc system headers
> > >...
> >
> > Why did the glibc 2.31 upgrade add a not upstreamed patch from 2017 that
> > created these conflicts?
> 
> This supports building userspace with clang better and find more
> errors when fortify sources option is on.

What "errors" are you referring to?
From a semantic point of view the code is correct.

This is a relict from K&R C that novice C programmers often misinterpret, 
but I'd say gcc made the right call by not including the warning for the
more general case in -Wall.

The cases with an actual problem are being caught by a different gcc 
warning that has been included in -Wall for decades.

> this patch was already proposed to glibc and I will follow up on it.

This is an area with different semantics in C and C++.
All your "fixes" indicate that the result is C++ semantics in C code.
Which is not something you can do in the public glibc headers.

> It definitely improves fortify when using clang

What is the point of all this when you "fix" the bogus error caused by 
this patch with a function attribute like in this case for gettext?

The gettext patch is simply wrong.

> > The commit message does not mention that this patch was added,
> > and an OE-only patch that makes a compiler reject valid C code
> > is not good.
>
> I think thats my bad as it slipped my mind with numerous rebases I did
> over the life of the glibc patchset.
>...

Was the glibc upgrade sent to the mailing list for review?

cu
Adrian