Re: [Bug-wget] Building and testing wget 1.16.1 on MinGW

[Eli Zaretskii]

> From: Tim Ruehsen <address@hidden>
> Date: Fri, 19 Dec 2014 12:53:13 +0100
> 
> >      warc.c: In function 'warc_write_warcinfo_record':
> >      warc.c:677:3: warning: passing argument 1 of 'strdup' makes pointer
> > from integer without a cast [enabled by default] In file included from
> > ../lib/string.h:27:0,
> >                   from
> > d:\usr\bin\../lib/gcc/mingw32/4.7.2/../../../../include/winnt.h:37, from
> > d:\usr\bin\../lib/gcc/mingw32/4.7.2/../../../../include/windef.h:253, from
> > d:\usr\bin\../lib/gcc/mingw32/4.7.2/../../../../include/windows.h:48, from
> > mswindows.h:44,
> >                   from sysdep.h:98,
> >                   from wget.h:47,
> >                   from warc.c:34:
> >      d:\usr\bin\../lib/gcc/mingw32/4.7.2/../../../../include/string.h:92:39:
> > note: expected 'const char *' but argument is of type 'int'
> > 
> >    This is because warc.c includes libgen.h only on non-Windows
> >    systems, and the prototype for basename is declared on MinGW's libgen.h.
> > 
> >    Proposed solution:
> > 
> > --- src/warc.c~0    2014-12-02 09:49:37.000000000 +0200
> > +++ src/warc.c      2014-12-19 12:16:25.827125000 +0200
> > @@ -54,10 +54,11 @@ as that of the covered work.  */
> >  #include <uuid.h>
> >  #endif
> > 
> > -#ifndef WINDOWS
> > -#include <libgen.h>
> > -#else
> > -#include <fcntl.h>
> > +#if !defined WINDOWS || defined __MINGW32__
> > +# include <libgen.h>
> > +#endif
> > +#ifdef WINDOWS
> > +# include <fcntl.h>
> >  #endif
> > 
> >  #include "warc.h"
> 
> The man page for basename says that there is a POSIX-2001 and a GNU version.
> The POSIX version does not allow string literals - the function writes into 
> the argument string :-(

The MinGW implementation indeed writes into its argument string.
However, warc.c already handles this contingency, and works on a copy
of the argument passed to warc_write_record.  So I see no problem
here.

> So I guess we should take basename from gnulib to have a consistent (and 
> graceful) behavior. 

See above: I don't see the need.

Moreover, I'm not sure we are talking about the same problem.  Note
that the compiler warning is about an argument being an 'int' whereas
strdup expected a 'char *'.  This is due to implicitly assuming the
return value is an 'int' when there's no prototype.  So the issue
isn't 'const char *' vs 'char *', the issue is that there was no
prototype for the compiler to use.

> Commands work either way:
> remoteencoding = UTF-8
> remote-encoding = UTF-8
> remote_encoding = UTF-8

This is not documented anywhere I could see (I suggest to document
that, perhaps in the same example file, if not in the Info manual).
In any case, the documented forms of the options use the underscore
'_', so I think so should the example file, for didactic reasons if
nothing else.

> > +#httpsonly = off  ??? doesn't seem to exist
> Only exists when HAVE_SSL is defined.

I have HAVE_SSL defined, see the configure report I posted.  But the
main issue here is that this option is not documented in the manual,
AFAICS.

> # to run a single test
> cd tests
> ./Test-N-old.px
> or 
> make check TESTS="Test-N-old"
> 
> If you want debugging output, I add -d directly into the .px file (my 
> $cmdline 
> = ...).

Thanks, this will help.

Btw, to debug the Test-N issue, I had to add the time stamps being
compared to the Perl script that runs the test.  It would be nice to
have that in the tests by default, because just by looking at the time
stamps, one can immediately understand in what direction to look for
the problem (in my case I saw a difference of 3600 sec).