Re: [Bug-wget] Remove most warnings for missing extern

[Darshit Shah]

On 11/21, Tim Rühsen wrote:
> On Friday 21 November 2014 19:58:46 Darshit Shah wrote:
> > On 11/21, Tim Rühsen wrote:
> > >On Friday 21 November 2014 17:13:22 Darshit Shah wrote:
> > >> Clang provides some warnings for missing extern declarations for
> > >> non-static
> > >> variables. The following two patches clear most of them. I can currently
> > >> see only more such warning which is caused by build_info.c. To fix this,
> > >> someone will have to hack on the build_info.px perl script.
> > >
> > >After ./boostrap
> > >./configure
> > >
> > >I get
> > >
> > >main.c:58:21: fatal error: version.h: No such file or directory
> > >
> > > #include "version.h"
> >
> > I just ran:
> > make maintainer-clean
> > ./bootstrap
> > ./configure
> > make
> >
> > and it works for me without any issues. Maybe you could try running a simple
> > make clean first? If it still doesn't work, I'll try looking into it again.
> > But since I can't reproduce it, its hard to guess where the error would be.
>
> Try to remove src/version.c and src/version.h. Than perform a 'make'.
I confirmed that both version.c and version.h didn't exist when I ran make.

I also tried again by simply by deleting version.* before running make again.
It still compiles perfectly for me.

> IMHO, version.c and version.h has wrong dependencies in Makefile.am.
> They depend on changed wget_SOURCES, which don't have to change.
I think version.c and version.h have the right dependencies. I'm not sure about 
the dependency on ../lib/libgnu.a, but the dependency on wget_SOURCES is 
perfect.

Let me start explaining my reasons with a question, when does version.c need to 
be re-created?

Version.c needs regeneration when any of its component strings have a change in
value.
The $COMPILE string changes when the libraries being used change, or more 
frequently, when the CFLAGS variable changes.
The link_string is dependent on the AM_CFLAGS, CFLAGS, LDFLAGS, AM_LDFLAGS and 
LIBS variables.
One last part that the version data is dependent on is the current patch level 
of Wget.

Now, within a Makefile, how do we determine these dependencies? Now, I'm not 
sure if commits that solely touch the non-C source files cause version.c to be 
regenerated. But all other commits will change files in wget_SOURCES. Hence 
forcing a change in version.c
Similarly, any change in the CFLAGS, LDFLAGS or LIBS variables will also cause 
files in the wget_SOURCES variable to be recompiled. As a result of that, 
version.c will be marked for re-compilation since its dependencies have changed. 
This is exactly how we want it to behave.

> I could make it work by using
> version.h: $(srcdir)/Makefile.am
> ...
> version.c: version.h
>
> This at least worked (after a ./config.status), but it may be wrong.
> What exactly has to change so that we need to rebuild version.[ch]  ?
> It's not the Wget sources !
In general, version.h doesn't need to be rebuilt. I think the best way is to 
explicitly write it and commit version.h into the source tree. It doesn't depend 
on any values that are generated at runtime. If everyone agrees on this setup, 
I'll gladly change the patch to simply statically include version.h. This does 
seem like a better and more efficient idea to me.

version.c on the other hand, does indeed need to be re-generated every time the 
wget_SOURCES are regenerated. Because it marks a change in either the patch 
level, or in one of the compilation variables which is other impossible to track 
from a Makefile.
> Tim
>
> > >Maybe you can also fix these warnings them before pushing:
> > >
> > >host.c: In function 'address_list_set_faulty':
> > >host.c:148:55: warning: unused parameter 'index' [-Wunused-parameter]
> > >
> > > address_list_set_faulty (struct address_list *al, int index)
> > >
> > >cookies.c: In function 'discard_matching_cookie':
> > >cookies.c:303:15: warning: variable 'res' set but not used
> > >[-Wunused-but-set- variable]
> > >
> > >           int res;
> >
> > It seems these warnings aren't enabled by -Wall and -Wextra on Clang. In
> > fact -Wunused-but-set-variable is GCC specific only. Which is why I never
> > saw these warnings in my output.
>
> There is also:
> utils.c: In function 'abort_run_with_timeout':
> utils.c:1919:29: warning: unused parameter 'sig' [-Wunused-parameter]
> abort_run_with_timeout (int sig)


--- end quoted text ---

--
Thanking You,
Darshit Shah

pgpZybcP3o3hX.pgp
Description: PGP signature