Hi Eli, On Friday 19 December 2014 17:02:53 Eli Zaretskii wrote: > > From: Tim Ruehsen <[email protected]> > > 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.
Right, we have two issues. But using basename from gnulib increases overall
compatibility *AND* also fixes your issue regarding libgen.h. We simply do not
need it any more when using gnulib.
The only question I have, is if we can drop #include <fcntl.h> for WINDOWS ?
I prepared a patch... please give it a try (you need ./bootstrap and
./configure after applying) and report back.
> > 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.
ACK. Either it becomes documented or the example wgetrc should use the
documented names.
> > > +#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.
From 'man wget' (man page and info page generated from wget.texi):
--https-only
When in recursive mode, only HTTPS links are followed.
> 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).
Since I am not 100% sure what you are talking about and you already extended
the (or a) Test-N, please send us diff/patch.
Tim
From 97abb9f63eb8fd2fbd5fbc9fabdbeccf37d59f5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim Rühsen?= <[email protected]> Date: Fri, 19 Dec 2014 16:07:38 +0100 Subject: [PATCH] gnulib: Use basename() from gnulib module 'dirname' Avoid basename incompatibilities between POSIX and GNU implementations. Also, libgen.h isn't needed any more which increases compatibility. --- bootstrap.conf | 1 + src/main.c | 7 ++----- src/warc.c | 16 +++------------- 3 files changed, 6 insertions(+), 18 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index 211f0ad..8d45475 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -36,6 +36,7 @@ c-strcasestr clock-time close connect +dirname fcntl fnmatch futimens diff --git a/src/main.c b/src/main.c index f511258..608a20c 100644 --- a/src/main.c +++ b/src/main.c @@ -56,6 +56,7 @@ as that of the covered work. */ #include "warc.h" #include "version.h" #include "c-strcase.h" +#include "dirname.h" #include <getopt.h> #include <getpass.h> #include <quote.h> @@ -1033,11 +1034,7 @@ main (int argc, char **argv) /* On VMS, lose the "dev:[dir]" prefix and the ".EXE;nnn" suffix. */ exec_name = vms_basename (argv[0]); #else /* def __VMS */ - exec_name = strrchr (argv[0], PATH_SEPARATOR); - if (!exec_name) - exec_name = argv[0]; - else - ++exec_name; + exec_name = basename (argv[0]); #endif /* def __VMS [else] */ #ifdef WINDOWS diff --git a/src/warc.c b/src/warc.c index 4959836..ef87f46 100644 --- a/src/warc.c +++ b/src/warc.c @@ -35,6 +35,7 @@ as that of the covered work. */ #include "hash.h" #include "utils.h" #include "version.h" +#include "dirname.h" #include <stdio.h> #include <stdlib.h> @@ -54,12 +55,6 @@ as that of the covered work. */ #include <uuid.h> #endif -#ifndef WINDOWS -#include <libgen.h> -#else -#include <fcntl.h> -#endif - #include "warc.h" #include "exits.h" @@ -671,7 +666,7 @@ warc_write_warcinfo_record (char *filename) { FILE *warc_tmp; char timestamp[22]; - char *filename_copy, *filename_basename; + char *filename_basename; /* Write warc-info record as the first record of the file. */ /* We add the record id of this info record to the other records in the @@ -681,8 +676,7 @@ warc_write_warcinfo_record (char *filename) warc_timestamp (timestamp, sizeof(timestamp)); - filename_copy = strdup (filename); - filename_basename = strdup (basename (filename_copy)); + filename_basename = basename (filename); warc_write_start_record (); warc_write_header ("WARC-Type", "warcinfo"); @@ -695,8 +689,6 @@ warc_write_warcinfo_record (char *filename) warc_tmp = warc_tempfile (); if (warc_tmp == NULL) { - xfree (filename_copy); - xfree (filename_basename); return false; } @@ -722,8 +714,6 @@ warc_write_warcinfo_record (char *filename) if (! warc_write_ok) logprintf (LOG_NOTQUIET, _("Error writing warcinfo record to WARC file.\n")); - xfree (filename_copy); - xfree (filename_basename); fclose (warc_tmp); return warc_write_ok; } -- 2.1.3
signature.asc
Description: This is a digitally signed message part.
