You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Karl Fogel <kf...@newton.ch.collab.net> on 2002/07/16 04:12:07 UTC

bogus compiler warnings in macros that take `socket'

When compiling Subversion, GCC now gives this warning whenever
apr_network_io.h is included:

   /usr/local/apache2/include/apr_network_io.h:718: warning: \
      declaration of `socket' shadows global declaration

The warnings result from these macros:

   APR_DECLARE_INHERIT_SET(socket);
   APR_DECLARE_INHERIT_UNSET(socket);

...which *look* like they're taking a parameter named `socket' and
(GCC presumes) using it as a variable within the macro expansion.  Of
course, we all know that's not what's happening -- instead, the macro
expands to a declaration that has "socket" as a substring in its name.

So it would be fair to call this a compiler bug, in that GCC is giving
overeager warnings.

Unfortunately, fixing this in GCC might be a lot of work; possibly a
*real lot*, depending on the amount of automated analysis required and
when/where it needs to happen.

Maybe we could just change APR?  I don't have a specific solution to
propose, as I haven't really looked at what-all the inherit_set/unset
stuff is doing; maybe someone who knows better could say what our
options are?

The warnings are a big problem; they cause us to miss real warnings
from our code, because we're getting used to skipping over these bogus
warnings.

-K

Re: bogus compiler warnings in macros that take `socket'

Posted by Cliff Woolley <jw...@virginia.edu>.
On Mon, 15 Jul 2002, Ian Holsman wrote:

> I'm doing it now..

I just finished it.  Where there's a will there's a way.  :)  About to
commit.


Re: bogus compiler warnings in macros that take `socket'

Posted by Ian Holsman <ia...@apache.org>.
Cliff Woolley wrote:
> On 15 Jul 2002, Karl Fogel wrote:
> 
> 
>>>commit it tonight.  we're about to do a .40 release in httpd, and
>>>I'll roll this into it.
>>
>>I unfortunately have other fish to fry tonight and won't be able to do
>>this.  Sorry about that... (Maybe you can take care of it, though?)
> 
> 
> Hmm... well if Ian wants it done tonight, I guess I can stay up a bit
> longer and take a crack at it.  If it compiles, it must work, right?
> hehe ;-)
> 
I'm doing it now..


> --Cliff
> 




Re: bogus compiler warnings in macros that take `socket'

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Cliff Woolley <jw...@virginia.edu> writes:
> Hmm... well if Ian wants it done tonight, I guess I can stay up a bit
> longer and take a crack at it.  

You're my hero.

> If it compiles, it must work, right? hehe ;-)

+shudder+

Re: bogus compiler warnings in macros that take `socket'

Posted by Cliff Woolley <jw...@virginia.edu>.
On Tue, 16 Jul 2002, Cliff Woolley wrote:

> Hmm... well if Ian wants it done tonight, I guess I can stay up a bit
> longer and take a crack at it.  If it compiles, it must work, right?

Sorry, scratch that.  I forgot I'm on my alternate notebook (left the main
one at work and shut it down before I left :-( ) so I have no access to my
apache sandboxes or private key.  :-]

I'll do it first thing in the AM unless somebody beats me to it.

--Cliff


Re: bogus compiler warnings in macros that take `socket'

Posted by Cliff Woolley <jw...@virginia.edu>.
On 15 Jul 2002, Karl Fogel wrote:

> > commit it tonight.  we're about to do a .40 release in httpd, and
> > I'll roll this into it.
>
> I unfortunately have other fish to fry tonight and won't be able to do
> this.  Sorry about that... (Maybe you can take care of it, though?)

Hmm... well if Ian wants it done tonight, I guess I can stay up a bit
longer and take a crack at it.  If it compiles, it must work, right?
hehe ;-)

--Cliff


Re: bogus compiler warnings in macros that take `socket'

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Ian Holsman <ia...@apache.org> writes:
> Karl
> commit it tonight.
> we're about to do a .40 release in httpd, and I'll roll this into it.

I unfortunately have other fish to fry tonight and won't be able to do
this.  Sorry about that... (Maybe you can take care of it, though?)

-K

Re: bogus compiler warnings in macros that take `socket'

Posted by Ian Holsman <ia...@apache.org>.
Karl Fogel wrote:
> Cliff Woolley <jw...@virginia.edu> writes:
> 
>>Rather than having a separate parameter to the macro, I think I'd rather
>>see something like this:
>>
>>#define APR_DECLARE_INHERIT_SET(name) \
>>    APR_DECLARE(apr_status_t) apr_##name##_inherit_set( \
>>                                             apr_##name##_t *the##name)
>>
>>thesocket, mysocket, socket_param, something like that.
> 
> 
> Much better solution!  I totally didn't think of that.
> 
> Unless anyone objects, I will be committing this sometime in the next
> few days.  (Unless you want it, Cliff, in which case it's all yours.)
> 
Karl
commit it tonight.
we're about to do a .40 release in httpd, and I'll roll this into it.

--Ian

> -K
> 




Re: bogus compiler warnings in macros that take `socket'

Posted by Cliff Woolley <jw...@virginia.edu>.
On 15 Jul 2002, Karl Fogel wrote:

> > thesocket, mysocket, socket_param, something like that.
>
> Much better solution!  I totally didn't think of that.
>
> Unless anyone objects, I will be committing this sometime in the next
> few days.  (Unless you want it, Cliff, in which case it's all yours.)

Have at it, I'm busy getting ready for SIGGRAPH.  :-)

--Cliff


Re: bogus compiler warnings in macros that take `socket'

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Cliff Woolley <jw...@virginia.edu> writes:
> Rather than having a separate parameter to the macro, I think I'd rather
> see something like this:
> 
> #define APR_DECLARE_INHERIT_SET(name) \
>     APR_DECLARE(apr_status_t) apr_##name##_inherit_set( \
>                                              apr_##name##_t *the##name)
> 
> thesocket, mysocket, socket_param, something like that.

Much better solution!  I totally didn't think of that.

Unless anyone objects, I will be committing this sometime in the next
few days.  (Unless you want it, Cliff, in which case it's all yours.)

-K

Re: bogus compiler warnings in macros that take `socket'

Posted by Cliff Woolley <jw...@virginia.edu>.
On 15 Jul 2002, Karl Fogel wrote:

>  #define APR_DECLARE_INHERIT_SET(name) \
>      APR_DECLARE(apr_status_t) apr_##name##_inherit_set(apr_##name##_t *name)

That's what I figured.  :-/

Rather than having a separate parameter to the macro, I think I'd rather
see something like this:

#define APR_DECLARE_INHERIT_SET(name) \
    APR_DECLARE(apr_status_t) apr_##name##_inherit_set( \
                                             apr_##name##_t *the##name)

thesocket, mysocket, socket_param, something like that.

--Cliff


Re: bogus compiler warnings in macros that take `socket'

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Cliff Woolley <jw...@virginia.edu> writes:
> This is not the kind of warning you would get at the preprocessor stage --
> you'll only get this after macro expansion has already occurred.  Sounds
> like a legitimate bug in APR to me.

Oh, I see it now.  The problem is that the expansions use the same
`name' for the parameter of the function:

 /**
  * @param name Set Inheritance for this Socket/File Handle
  */
 #define APR_DECLARE_INHERIT_SET(name) \
     APR_DECLARE(apr_status_t) apr_##name##_inherit_set(apr_##name##_t *name)
   
 /**
  * @param name Unset Inheritance for this Socket/File Handle
  */
 #define APR_DECLARE_INHERIT_UNSET(name) \
     APR_DECLARE(apr_status_t) apr_##name##_inherit_unset(apr_##name##_t *name)
   
Bleargh.  I guess we should just make them take a second parameter, to
be used for the expanded parameter name.  For example:

 /**
  * @param name Set Inheritance for this Socket/File Handle
  */
 #define APR_DECLARE_INHERIT_SET(name, param) \
     APR_DECLARE(apr_status_t) apr_##name##_inherit_set(apr_##name##_t *param)

...change the call, and then go change the function defs in (say)
sockets.c.  That's a pain because we have to do the same to the
APR_IMPLEMENT_INHERIT_SET and APR_IMPLEMENT_INHERIT_UNSET macros in
the arch-specific inherit.h's.

But I don't see a better way.  Unless... <evil grin>...

Unless we just rename the parameter to "sock".  Then we have to change
apr_socket_t to apr_sock_t, and the interfaces to apr_sock_*().  But
if we're willing to do those things, then this change is very easy to
make.

Thoughts?,
-Karl


Re: bogus compiler warnings in macros that take `socket'

Posted by Cliff Woolley <jw...@virginia.edu>.
On Mon, 15 Jul 2002, Karl Fogel wrote:

>    APR_DECLARE_INHERIT_SET(socket);
>    APR_DECLARE_INHERIT_UNSET(socket);
>
> So it would be fair to call this a compiler bug, in that GCC is giving
> overeager warnings.
>
> Unfortunately, fixing this in GCC might be a lot of work; possibly a
> *real lot*, depending on the amount of automated analysis required and
> when/where it needs to happen.

This is not the kind of warning you would get at the preprocessor stage --
you'll only get this after macro expansion has already occurred.  Sounds
like a legitimate bug in APR to me.

--Cliff