[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 08 of 22] eliminated gcc-4 compilation warnings



On Thu  6.Nov'08 at 12:30:19 -0800, John H. Robinson, IV wrote:
> Carlos R. Mafra wrote:
> > John H. Robinson, IV wrote:
> > > # HG changeset patch
> > > # User Dan Pascu <dan@ag-projects.com>
> > > # Date 1124755099 25200
> > > # Branch wm_0_92
> > > # Node ID 345324c47d750b7f1847e926c50f8e044eb4a24f
> > > # Parent  6f8b21fc008fe3618a793761b2a9191bb433140f
> > 
> > 
> > > eliminated gcc-4 compilation warnings
> > 
> > Ok, so I have a problem with commit logs like this! :-)
> 
> It was written by Dan when he commited it to the old CVS tre. I pulled
> it straight from there.
> 
> > IMHO a commit which fixes compilation warnings should
> > explicitly mention what those warnings are in the first
> > place. That way people trying to review it know what
> > the intentions were from the start.
> > 
> > I say this because I got curious about how to fix compilation
> > warnings in general and wanted to learn from other people fixes,
> > especially the ones which are highly experienced like Dan.
> > 
> > Ok, so not knowing what the warnings were I simply reverted
> > this commit in my git tree and compiled it again (using
> > gcc 4.2.3 in 64-bit cpu). For my surprise only one warning
> > was added this way (of course it may add more warnings with
> > other gcc versions and/or 32-bit/64-bit)
> > 
> > startup.c: In function 'StartUp':
> > startup.c:613: warning: incompatible implicit declaration of built-in function 'memset'
> > 
> > which is fixed by this particular change here
> > 
> > > --- a/src/startup.c	Mon Aug 22 12:07:57 2005 -0700
> > > +++ b/src/startup.c	Mon Aug 22 16:58:19 2005 -0700
> > > @@ -25,6 +25,7 @@
> > >  #include <stdio.h>
> > >  #include <stdlib.h>
> > >  #include <unistd.h>
> > > +#include <string.h>
> > >  #include <errno.h>
> > >  #include <signal.h>
> > >  #include <sys/wait.h>
> 
> I'l look into getting the above patch applied.

Hm, I think I didn't express myself well.

The patch must be ok, as it was Dan who wrote it. And you
have already added it to the repo.

I was just disappointed by the fact that I didn't see what
those warnings he said he was fixing were. So I reverted
his patch and compiled wmaker again expecting to see the
warnings which were fixed by the patch, but only one
such warning came up. The one in startup.c:613, which
is indeed fixed by that part of Dan's patch which I quoted.

So in summary: You don't need to do anything :-)

I was just curious about the warnings and thought it
would be a good motivation to review his patch, but
I couldn't reproduce the warnings here (which is
not unexpected, as I have a different gcc, different
cpu etc).



-- 
To unsubscribe, send mail to wmaker-dev-unsubscribe@lists.windowmaker.info.