Re: [Bug-wget] [PATCH] Fixing C89 warnings
That's really good news to me. But there are still lots
more C99 errors. Espesially in main.c.
Now another C99 error got into openssl.c. Patch:
--- Git-latest/src/openssl.c2014-12-03 14:06:19 +
+++ openssl.c 2014-12-03 14:14:37 +
@@ -170,6 +170,7 @@
ssl_init (void)
{
SSL_METHOD const *meth;
+ long ssl_options = 0;
#if OPENSSL_VERSION_NUMBER = 0x00907000
if (ssl_true_initialized == 0)
@@ -203,8 +204,6 @@
SSLeay_add_all_algorithms ();
SSLeay_add_ssl_algorithms ();
- long ssl_options = 0;
-
switch (opt.secure_protocol)
{
#ifndef OPENSSL_NO_SSL2
---
FYI. There are gcc options to trigger an error for these
cases. Such as gcc -Wdeclaration-after-statement -Werror.
But there are other harmless warnings.
--
--gv
Re: [Bug-wget] [PATCH] Fixing C89 warnings
Gisle Vanem gva...@yahoo.no writes:
That's really good news to me. But there are still lots
more C99 errors. Espesially in main.c.
Now another C99 error got into openssl.c. Patch:
Heh, when I patched wget-1.16 I took example on the code around to
decide where to declare the variable. And indeed the meth variable
decl had since moved in the git repo. Sorry for not doing my
homework. :)
--- Git-latest/src/openssl.c2014-12-03 14:06:19 +
+++ openssl.c 2014-12-03 14:14:37 +
@@ -170,6 +170,7 @@
ssl_init (void)
{
SSL_METHOD const *meth;
+ long ssl_options = 0;
#if OPENSSL_VERSION_NUMBER = 0x00907000
if (ssl_true_initialized == 0)
@@ -203,8 +204,6 @@
SSLeay_add_all_algorithms ();
SSLeay_add_ssl_algorithms ();
- long ssl_options = 0;
-
switch (opt.secure_protocol)
{
#ifndef OPENSSL_NO_SSL2
---
FYI. There are gcc options to trigger an error for these
cases. Such as gcc -Wdeclaration-after-statement -Werror.
But there are other harmless warnings.
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: [Bug-wget] [PATCH] Fixing C89 warnings
Thanks, Gisle.
It is pushed.
That's really good news to me. But there are still lots
more C99 errors. Espesially in main.c.
Feel free to send patches.
Sadly, I can't work with -Werror (but use -std=c89 -pedantic -Wall -Wextra +
more). And I also might oversee warnings because the scrolling is too fast.
Thanks to have a thorough look at current changes.
Tim
Am Mittwoch, 3. Dezember 2014, 13:41:13 schrieb Gisle Vanem:
That's really good news to me. But there are still lots
more C99 errors. Espesially in main.c.
Now another C99 error got into openssl.c. Patch:
--- Git-latest/src/openssl.c2014-12-03 14:06:19 +
+++ openssl.c 2014-12-03 14:14:37 +
@@ -170,6 +170,7 @@
ssl_init (void)
{
SSL_METHOD const *meth;
+ long ssl_options = 0;
#if OPENSSL_VERSION_NUMBER = 0x00907000
if (ssl_true_initialized == 0)
@@ -203,8 +204,6 @@
SSLeay_add_all_algorithms ();
SSLeay_add_ssl_algorithms ();
- long ssl_options = 0;
-
switch (opt.secure_protocol)
{
#ifndef OPENSSL_NO_SSL2
---
FYI. There are gcc options to trigger an error for these
cases. Such as gcc -Wdeclaration-after-statement -Werror.
But there are other harmless warnings.
signature.asc
Description: This is a digitally signed message part.
Re: [Bug-wget] [PATCH] Fixing C89 warnings
On 11/20, Tim Rühsen wrote:
On Thursday 20 November 2014 11:52:40 Darshit Shah wrote:
On 11/19, Tim Rühsen wrote:
Am Mittwoch, 19. November 2014, 22:07:28 schrieb Darshit Shah:
On 11/18, Tim Rühsen wrote:
This patch fixes most C89 warnings for me (-std=c89 -pedantic) since
these
may prevent from compiling with MSVC.
There are still some warnings ISO C forbids conversion of function
pointer
to object pointer type [-Wpedantic] left over. I'll care for these the
next days. There are architectures where function pointers have a
different size from void *. That's why this warning has a meaning.
Tim
From 11baace21e1fa04a92baa395f38ebad8001e9762 Mon Sep 17 00:00:00 2001
From: Tim Ruehsen tim.rueh...@gmx.de
Date: Tue, 18 Nov 2014 22:00:48 +0100
Subject: [PATCH] Trivial fixes for C89 compliancy.
snip
diff --git a/src/gnutls.c b/src/gnutls.c
index 87d1d0b..42201e5 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -54,6 +54,10 @@ as that of the covered work. */
# include w32sock.h
#endif
+#ifdef HAVE_ALLOCA_H
+# include alloca.h
+#endif
+
#include host.h
static int
@@ -122,9 +126,10 @@ ssl_init (void)
while ((dent = readdir (dir)) != NULL)
{
struct stat st;
- char ca_file[dirlen + strlen(dent-d_name) + 2];
+ size_t ca_file_length = dirlen + strlen(dent-d_name) +
2;
+ char *ca_file = alloca(ca_file_length);
What happens when HAVE_ALLOCA_H is not defined? The above code will
attempt
to call a function that does not exist and Wget will crash.
I think, we can simply malloc / free() these. Sure alloca is more
convenient, but we need a valid fallback for when it is not available.
There are systems without alloca.h header file. Thus the check.
(I just had an error report for libpsl with this issue.)
My question was. On systems where alloca.h is not available, this code has
no fallback. If alloca.h is not included, then the
char *ca_file = alloca(ca_file_length);
line will prevent the source from compiling. Should be not have something to
prevent that?
Sorry for being unclear. NOT having alloca.h means, alloca() is defined
somewhere else, or it is even interpreted directly by the compiler. As long as
you don't use -Werror, you will just see a warning when compiling.
Okay. I wasn't aware that alloca() as a function will still be available. I have
always relied on including alloca.h. If it is how you say, then this patch looks
fine to me.
Conclusion: I do not see a problem here. If you think there is, we should
completely repair Wget. But this is a different issue. This patch will lean on
the existing Wget standards.
I do not know systems without alloca() function.
Wget sources already use alloca on several places.
If Wget sources are already doing this, and no one has yet complained then
it means we've probably not hit the case yet where someone tried to compile
Wget on a system without alloca.h
That is not true, see above.
But I will amend the patch and remove #include alloca.h. I just saw, wget.h
already includes it.
Please push it after this change.
Tim
--
Thanking You,
Darshit Shah
pgpy3Y1iKB_KM.pgp
Description: PGP signature
Re: [Bug-wget] [PATCH] Fixing C89 warnings
On Thursday 20 November 2014 14:08:27 Darshit Shah wrote:
On 11/20, Tim Rühsen wrote:
On Thursday 20 November 2014 11:52:40 Darshit Shah wrote:
On 11/19, Tim Rühsen wrote:
Am Mittwoch, 19. November 2014, 22:07:28 schrieb Darshit Shah:
On 11/18, Tim Rühsen wrote:
This patch fixes most C89 warnings for me (-std=c89 -pedantic) since
these
may prevent from compiling with MSVC.
There are still some warnings ISO C forbids conversion of function
pointer
to object pointer type [-Wpedantic] left over. I'll care for these
the
next days. There are architectures where function pointers have a
different size from void *. That's why this warning has a meaning.
Tim
From 11baace21e1fa04a92baa395f38ebad8001e9762 Mon Sep 17 00:00:00
2001
From: Tim Ruehsen tim.rueh...@gmx.de
Date: Tue, 18 Nov 2014 22:00:48 +0100
Subject: [PATCH] Trivial fixes for C89 compliancy.
snip
diff --git a/src/gnutls.c b/src/gnutls.c
index 87d1d0b..42201e5 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -54,6 +54,10 @@ as that of the covered work. */
# include w32sock.h
#endif
+#ifdef HAVE_ALLOCA_H
+# include alloca.h
+#endif
+
#include host.h
static int
@@ -122,9 +126,10 @@ ssl_init (void)
while ((dent = readdir (dir)) != NULL)
{
struct stat st;
- char ca_file[dirlen + strlen(dent-d_name) + 2];
+ size_t ca_file_length = dirlen + strlen(dent-d_name)
+
2;
+ char *ca_file = alloca(ca_file_length);
What happens when HAVE_ALLOCA_H is not defined? The above code will
attempt
to call a function that does not exist and Wget will crash.
I think, we can simply malloc / free() these. Sure alloca is more
convenient, but we need a valid fallback for when it is not available.
There are systems without alloca.h header file. Thus the check.
(I just had an error report for libpsl with this issue.)
My question was. On systems where alloca.h is not available, this code
has
no fallback. If alloca.h is not included, then the
char *ca_file = alloca(ca_file_length);
line will prevent the source from compiling. Should be not have something
to prevent that?
Sorry for being unclear. NOT having alloca.h means, alloca() is defined
somewhere else, or it is even interpreted directly by the compiler. As long
as you don't use -Werror, you will just see a warning when compiling.
Okay. I wasn't aware that alloca() as a function will still be available. I
have always relied on including alloca.h. If it is how you say, then this
patch looks fine to me.
Conclusion: I do not see a problem here. If you think there is, we should
completely repair Wget. But this is a different issue. This patch will lean
on the existing Wget standards.
I do not know systems without alloca() function.
Wget sources already use alloca on several places.
If Wget sources are already doing this, and no one has yet complained
then
it means we've probably not hit the case yet where someone tried to
compile
Wget on a system without alloca.h
That is not true, see above.
But I will amend the patch and remove #include alloca.h. I just saw,
wget.h already includes it.
Please push it after this change.
Pushed.
Tim
signature.asc
Description: This is a digitally signed message part.
Re: [Bug-wget] [PATCH] Fixing C89 warnings
On Wednesday 19 November 2014 22:51:10 Gisle Vanem wrote:
Tim Rühsen wrote:
This patch fixes most C89 warnings for me (-std=c89 -pedantic) since these
may prevent from compiling with MSVC.
That's really good news to me. But there are still lots
more C99 errors. Espesially in main.c.
I just know of
main.c:226:33: warning: ISO C forbids conversion of function pointer to object
pointer type [-Wpedantic]
{ help, 'h', OPT_FUNCALL, (void *)print_help, no_argument },
^
main.c:312:36: warning: ISO C forbids conversion of function pointer to object
pointer type [-Wpedantic]
{ version, 'V', OPT_FUNCALL, (void *) print_version, no_argument },
^
main.c: In function 'main':
main.c:1163:35: warning: ISO C forbids conversion of object pointer to
function pointer type [-Wpedantic]
void (*func) (void) = (void (*) (void)) cmdopt-data;
And either I or Darshit care about it.
Please 'git pull' and send the (error/warning) output of MSVC compilation. I
would like to know exactly what it complains about.
Tim
signature.asc
Description: This is a digitally signed message part.
Re: [Bug-wget] [PATCH] Fixing C89 warnings
Hi Gisle, please always answer to the mailing list. You don't have the latest git version, at least your diff isn't based on it. I pushed the C89 changes right before I asked you to give me the compiler output. I guess MSVC 16 can be used on WinXP while MSVC 18 is for Win7 and up ? Please redo your diff and check if the errors below still exist (looks like size_t is not available !?). If they do, please add your src/config.h. Tim On Thursday 20 November 2014 11:14:29 Gisle Vanem wrote: Please 'git pull' and send the (error/warning) output of MSVC compilation. I would like to know exactly what it complains about. First I've used MSVC v18 with no problem. But the MSVC v16 has no C99 feature like declaration after code. So in e.g. main.c and warc.c there are tons of error like these: cl -nologo -MD -O2 -Zi -W3 -I. -Ig:/MingW32/src/gnu/gnulib/lib -DENABLE_IPV6 -DENABLE_DEBUG -DENABLE_OPIE -DENABLE_DIGEST -DHAVE_LIBZ -DENABLE_WARC -D_WIN32_WINNT=0x0501 -DCTRLBREAK_BACKGND -Fo./MSVC_obj/warc.obj -c warc.c warc.c warc.c(171) : error C2275: 'size_t' : illegal use of this type as an expression g:\vc_2010\vc\include\codeanalysis\sourceannotations.h(29) : see declaration of 'size_t' warc.c(171) : error C2146: syntax error : missing ';' before identifier 'n' warc.c(171) : error C2065: 'n' : undeclared identifier warc.c(172) : error C2065: 'n' : undeclared identifier warc.c(172) : error C2065: 'n' : undeclared identifier warc.c(271) : error C2143: syntax error : missing ';' before 'type' warc.c(272) : error C2275: 'size_t' : illegal use of this type as an expression g:\vc_2010\vc\include\codeanalysis\sourceannotations.h(29) : see declaration of 'size_t' warc.c(272) : error C2146: syntax error : missing ';' before identifier 's' warc.c(272) : error C2065: 's' : undeclared identifier warc.c(273) : error C2065: 's' : undeclared identifier -- Too many so I was forced to create a fake-warc.c in my Makefile. I don't care about WARC either. But the rest of my diffs to make it compile with MSVC v16 are attached. signature.asc Description: This is a digitally signed message part.
Re: [Bug-wget] [PATCH] Fixing C89 warnings
Tim Ruehsen wrote:
You don't have the latest git version, at least your diff isn't based on it.
Okay. I was too quick. It builds fine with MSVC v16 except
for http.c and the rubbish in progress.c. See [1] below.
Diffs for http.c:
--- ../Git-latest/src/http.c2014-11-20 15:39:55 +
+++ ./http.c2014-11-20 15:54:05 +
@@ -1191,8 +1191,9 @@
parse_content_disposition (const char *hdr, char **filename)
{
param_token name, value;
- *filename = NULL;
bool is_url_encoded = false;
+
+ *filename = NULL;
for ( ; extract_param (hdr, name, value, ';', is_url_encoded);
is_url_encoded = false)
I guess MSVC 16 can be used on WinXP while MSVC 18 is for Win7 and up ?
Correct. I do have another PC with Win 8.1 and VC Express 2013
(MSVC v18), but feel more at ease with Win-XP ATM.
Please redo your diff and check if the errors below still exist (looks like
size_t is not available !?).
No problem with 'size_t' on MSVC v16.
[1]:
#else
# define count_cols(mbs) ((int)(strlen(mbs)))
# define cols_to_bytes(mbs, cols, *ncols) do { \
*ncols = cols; \
bytes = cols; \
}while (0)
#endif
(I forgot to add 'HAVE_WCWIDTH' and 'HAVE_MBTOWC').
And BTW. Since large-files are default on Windows (wgint is
__int64), this patch should be applied:
--- ../Git-latest/src/build_info.c.in 2014-10-30 13:33:41 +
+++ ./build_info.c.in 2014-11-05 13:28:49 +
@@ -2,11 +2,12 @@
https defined HAVE_SSL
ipv6defined ENABLE_IPV6
iri defined ENABLE_IRI
-large-file SIZEOF_OFF_T = 8
+large-file SIZEOF_OFF_T = 8 || defined WINDOWS
--gv
Re: [Bug-wget] [PATCH] Fixing C89 warnings
On Thursday 20 November 2014 15:06:26 Gisle Vanem wrote:
Tim Ruehsen wrote:
You don't have the latest git version, at least your diff isn't based on
it.
Okay. I was too quick. It builds fine with MSVC v16 except
for http.c and the rubbish in progress.c. See [1] below.
Diffs for http.c:
That are good news.
--- ../Git-latest/src/http.c2014-11-20 15:39:55 +
+++ ./http.c2014-11-20 15:54:05 +
@@ -1191,8 +1191,9 @@
parse_content_disposition (const char *hdr, char **filename)
{
param_token name, value;
- *filename = NULL;
bool is_url_encoded = false;
+
+ *filename = NULL;
for ( ; extract_param (hdr, name, value, ';', is_url_encoded);
is_url_encoded = false)
I pushed it.
[1]:
#else
# define count_cols(mbs) ((int)(strlen(mbs)))
# define cols_to_bytes(mbs, cols, *ncols) do { \
*ncols = cols; \
bytes = cols; \
}while (0)
#endif
(I forgot to add 'HAVE_WCWIDTH' and 'HAVE_MBTOWC').
? could you send a patch ? I am not sure what to fix here.
And BTW. Since large-files are default on Windows (wgint is
__int64), this patch should be applied:
--- ../Git-latest/src/build_info.c.in 2014-10-30 13:33:41 +
+++ ./build_info.c.in 2014-11-05 13:28:49 +
@@ -2,11 +2,12 @@
https defined HAVE_SSL
ipv6defined ENABLE_IPV6
iri defined ENABLE_IRI
-large-file SIZEOF_OFF_T = 8
+large-file SIZEOF_OFF_T = 8 || defined WINDOWS
Does this hold true for Win32 (WinXP 32bit) ?
Or do we have to amend this check ?
Tim
signature.asc
Description: This is a digitally signed message part.
Re: [Bug-wget] [PATCH] Fixing C89 warnings
Tim Ruehsen wrote:
[1]:
#else
# define count_cols(mbs) ((int)(strlen(mbs)))
# define cols_to_bytes(mbs, cols, *ncols) do { \
*ncols = cols; \
bytes = cols; \
}while (0)
#endif
(I forgot to add 'HAVE_WCWIDTH' and 'HAVE_MBTOWC').
? could you send a patch ? I am not sure what to fix here.
FYI. the error from MSVC was:
progress.c(844) : error C2010: '*' : unexpected in macro formal parameter list
progress.c(978) : error C2059: syntax error : 'do'
Here is a patch:
--- ../Git-latest/src/progress.c2014-11-20 15:39:55 +
+++ progress.c 2014-11-20 16:44:03 +
@@ -841,10 +841,7 @@
}
#else
# define count_cols(mbs) ((int)(strlen(mbs)))
-# define cols_to_bytes(mbs, cols, *ncols) do { \
-*ncols = cols; \
-bytes = cols; \
-}while (0)
+# define cols_to_bytes(mbs, cols, ncols) *ncols = cols
#endif
Does this hold true for Win32 (WinXP 32bit) ?
Or do we have to amend this check ?
Windows since way back has supported 4 GB files. It's
been compilers that were slow following that. Since
MingW/MSVC have libc support for huge-files, that
'wgint' is hardcoded to 64 bits signed. I vaguely remember
me an Hrvoje discussed this long before you switched to
Gnulib.
--
--gv
Re: [Bug-wget] [PATCH] Fixing C89 warnings
Tim Rühsen tim.rueh...@gmx.de writes:
Am Mittwoch, 19. November 2014, 22:07:28 schrieb Darshit Shah:
On 11/18, Tim Rühsen wrote:
This patch fixes most C89 warnings for me (-std=c89 -pedantic) since these
may prevent from compiling with MSVC.
There are still some warnings ISO C forbids conversion of function pointer
to object pointer type [-Wpedantic] left over. I'll care for these the
next days. There are architectures where function pointers have a
different size from void *. That's why this warning has a meaning.
Tim
From 11baace21e1fa04a92baa395f38ebad8001e9762 Mon Sep 17 00:00:00 2001
From: Tim Ruehsen tim.rueh...@gmx.de
Date: Tue, 18 Nov 2014 22:00:48 +0100
Subject: [PATCH] Trivial fixes for C89 compliancy.
snip
diff --git a/src/gnutls.c b/src/gnutls.c
index 87d1d0b..42201e5 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -54,6 +54,10 @@ as that of the covered work. */
# include w32sock.h
#endif
+#ifdef HAVE_ALLOCA_H
+# include alloca.h
+#endif
+
#include host.h
static int
@@ -122,9 +126,10 @@ ssl_init (void)
while ((dent = readdir (dir)) != NULL)
{
struct stat st;
- char ca_file[dirlen + strlen(dent-d_name) + 2];
+ size_t ca_file_length = dirlen + strlen(dent-d_name) + 2;
+ char *ca_file = alloca(ca_file_length);
What happens when HAVE_ALLOCA_H is not defined? The above code will attempt
to call a function that does not exist and Wget will crash.
I think, we can simply malloc / free() these. Sure alloca is more
convenient, but we need a valid fallback for when it is not available.
There are systems without alloca.h header file. Thus the check.
(I just had an error report for libpsl with this issue.)
I do not know systems without alloca() function.
Wget sources already use alloca on several places.
I also think we should simply use malloc/free instead of alloca (not
that it is worth a rewriting now) but in future let's not add more
instances of it. malloc is safer than alloca.
Regards,
Giuseppe
Re: [Bug-wget] [PATCH] Fixing C89 warnings
On 20/11/14 16:56, Gisle Vanem wrote:
FYI. the error from MSVC was:
progress.c(844) : error C2010: '*' : unexpected in macro formal
parameter list
progress.c(978) : error C2059: syntax error : 'do'
Here is a patch:
--- ../Git-latest/src/progress.c2014-11-20 15:39:55 +
+++ progress.c 2014-11-20 16:44:03 +
@@ -841,10 +841,7 @@
}
#else
# define count_cols(mbs) ((int)(strlen(mbs)))
-# define cols_to_bytes(mbs, cols, *ncols) do { \
-*ncols = cols; \
-bytes = cols; \
-}while (0)
+# define cols_to_bytes(mbs, cols, ncols) *ncols = cols
#endif
I suggested this same change last month (2014-10-11). ACK
Re: [Bug-wget] [PATCH] Fixing C89 warnings
Am Donnerstag, 20. November 2014, 15:56:00 schrieb Gisle Vanem:
Tim Ruehsen wrote:
[1]:
#else
# define count_cols(mbs) ((int)(strlen(mbs)))
# define cols_to_bytes(mbs, cols, *ncols) do { \
*ncols = cols; \
bytes = cols; \
}while (0)
#endif
(I forgot to add 'HAVE_WCWIDTH' and 'HAVE_MBTOWC').
? could you send a patch ? I am not sure what to fix here.
FYI. the error from MSVC was:
progress.c(844) : error C2010: '*' : unexpected in macro formal parameter
list progress.c(978) : error C2059: syntax error : 'do'
Here is a patch:
--- ../Git-latest/src/progress.c2014-11-20 15:39:55 +
+++ progress.c 2014-11-20 16:44:03 +
@@ -841,10 +841,7 @@
}
#else
# define count_cols(mbs) ((int)(strlen(mbs)))
-# define cols_to_bytes(mbs, cols, *ncols) do { \
-*ncols = cols; \
-bytes = cols; \
-}while (0)
+# define cols_to_bytes(mbs, cols, ncols) *ncols = cols
#endif
Does this hold true for Win32 (WinXP 32bit) ?
Or do we have to amend this check ?
Windows since way back has supported 4 GB files. It's
been compilers that were slow following that. Since
MingW/MSVC have libc support for huge-files, that
'wgint' is hardcoded to 64 bits signed. I vaguely remember
me an Hrvoje discussed this long before you switched to
Gnulib.
Instead of a #define to replace a function, I decided to write two small
'dummy' functions here. It's pushed now.
I also pushed your patch/suggestion to always assume large-file for (defined
WINDOWS) in src/build_info.c.in.
Tim
signature.asc
Description: This is a digitally signed message part.
Re: [Bug-wget] [PATCH] Fixing C89 warnings
On 11/20, Ángel González wrote:
On 20/11/14 16:56, Gisle Vanem wrote:
FYI. the error from MSVC was:
progress.c(844) : error C2010: '*' : unexpected in macro formal
parameter list
progress.c(978) : error C2059: syntax error : 'do'
Here is a patch:
--- ../Git-latest/src/progress.c2014-11-20 15:39:55 +
+++ progress.c 2014-11-20 16:44:03 +
@@ -841,10 +841,7 @@
}
#else
# define count_cols(mbs) ((int)(strlen(mbs)))
-# define cols_to_bytes(mbs, cols, *ncols) do { \
-*ncols = cols; \
-bytes = cols; \
-}while (0)
+# define cols_to_bytes(mbs, cols, ncols) *ncols = cols
#endif
I suggested this same change last month (2014-10-11). ACK
And I was under the impression that we'd pushed that patch.
Anyways, thanks Tim for fixing it. I'd have preferred the macro expansions
because these functions will be called a very large number of times when
updating the progress bar. And simple things should ideally just get inlined.
--
Thanking You,
Darshit Shah
pgpTBCS9k8XMG.pgp
Description: PGP signature
Re: [Bug-wget] [PATCH] Fixing C89 warnings
;-) From my post: This patch fixes most C89 warnings for me (-std=c89 -pedantic) since these If the patch in my original post is OK for you, I'll would like to push it. Tim On Wednesday 19 November 2014 07:17:35 Darshit Shah wrote: I think we use std=gnu89 or std=gnu99. I've been using these and get no warnings on clang anymore. What configure command and CFLAGS are you using? I'd like to reproduce these and try to fix them. Thanking You, Darshit Shah Sent from mobile device. Please excuse my brevity On 19-Nov-2014 2:35 am, Tim Rühsen tim.rueh...@gmx.de wrote: This patch fixes most C89 warnings for me (-std=c89 -pedantic) since these may prevent from compiling with MSVC. There are still some warnings ISO C forbids conversion of function pointer to object pointer type [-Wpedantic] left over. I'll care for these the next days. There are architectures where function pointers have a different size from void *. That's why this warning has a meaning. Tim signature.asc Description: This is a digitally signed message part.
Re: [Bug-wget] [PATCH] Fixing C89 warnings
On 11/18, Tim Rühsen wrote:
This patch fixes most C89 warnings for me (-std=c89 -pedantic) since these may
prevent from compiling with MSVC.
There are still some warnings ISO C forbids conversion of function pointer to
object pointer type [-Wpedantic] left over. I'll care for these the next
days. There are architectures where function pointers have a different size
from void *. That's why this warning has a meaning.
Tim
From 11baace21e1fa04a92baa395f38ebad8001e9762 Mon Sep 17 00:00:00 2001
From: Tim Ruehsen tim.rueh...@gmx.de
Date: Tue, 18 Nov 2014 22:00:48 +0100
Subject: [PATCH] Trivial fixes for C89 compliancy.
snip
diff --git a/src/gnutls.c b/src/gnutls.c
index 87d1d0b..42201e5 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -54,6 +54,10 @@ as that of the covered work. */
# include w32sock.h
#endif
+#ifdef HAVE_ALLOCA_H
+# include alloca.h
+#endif
+
#include host.h
static int
@@ -122,9 +126,10 @@ ssl_init (void)
while ((dent = readdir (dir)) != NULL)
{
struct stat st;
- char ca_file[dirlen + strlen(dent-d_name) + 2];
+ size_t ca_file_length = dirlen + strlen(dent-d_name) + 2;
+ char *ca_file = alloca(ca_file_length);
What happens when HAVE_ALLOCA_H is not defined? The above code will attempt to
call a function that does not exist and Wget will crash.
I think, we can simply malloc / free() these. Sure alloca is more convenient,
but we need a valid fallback for when it is not available.
- snprintf (ca_file, sizeof(ca_file), %s/%s, ca_directory,
dent-d_name);
+ snprintf (ca_file, ca_file_length, %s/%s, ca_directory,
dent-d_name);
if (stat (ca_file, st) != 0)
continue;
The rest of the changes look pretty standard and harmless. I'm fine with this
patch. Push it once this alloca business has been sorted out.
--
Thanking You,
Darshit Shah
pgpT1GdjZxR2o.pgp
Description: PGP signature
Re: [Bug-wget] [PATCH] Fixing C89 warnings
Am Mittwoch, 19. November 2014, 22:07:28 schrieb Darshit Shah:
On 11/18, Tim Rühsen wrote:
This patch fixes most C89 warnings for me (-std=c89 -pedantic) since these
may prevent from compiling with MSVC.
There are still some warnings ISO C forbids conversion of function pointer
to object pointer type [-Wpedantic] left over. I'll care for these the
next days. There are architectures where function pointers have a
different size from void *. That's why this warning has a meaning.
Tim
From 11baace21e1fa04a92baa395f38ebad8001e9762 Mon Sep 17 00:00:00 2001
From: Tim Ruehsen tim.rueh...@gmx.de
Date: Tue, 18 Nov 2014 22:00:48 +0100
Subject: [PATCH] Trivial fixes for C89 compliancy.
snip
diff --git a/src/gnutls.c b/src/gnutls.c
index 87d1d0b..42201e5 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -54,6 +54,10 @@ as that of the covered work. */
# include w32sock.h
#endif
+#ifdef HAVE_ALLOCA_H
+# include alloca.h
+#endif
+
#include host.h
static int
@@ -122,9 +126,10 @@ ssl_init (void)
while ((dent = readdir (dir)) != NULL)
{
struct stat st;
- char ca_file[dirlen + strlen(dent-d_name) + 2];
+ size_t ca_file_length = dirlen + strlen(dent-d_name) + 2;
+ char *ca_file = alloca(ca_file_length);
What happens when HAVE_ALLOCA_H is not defined? The above code will attempt
to call a function that does not exist and Wget will crash.
I think, we can simply malloc / free() these. Sure alloca is more
convenient, but we need a valid fallback for when it is not available.
There are systems without alloca.h header file. Thus the check.
(I just had an error report for libpsl with this issue.)
I do not know systems without alloca() function.
Wget sources already use alloca on several places.
Tim
signature.asc
Description: This is a digitally signed message part.
Re: [Bug-wget] [PATCH] Fixing C89 warnings
Tim Rühsen wrote: This patch fixes most C89 warnings for me (-std=c89 -pedantic) since these may prevent from compiling with MSVC. That's really good news to me. But there are still lots more C99 errors. Espesially in main.c. --gv
Re: [Bug-wget] [PATCH] Fixing C89 warnings
On 11/19, Tim Rühsen wrote:
Am Mittwoch, 19. November 2014, 22:07:28 schrieb Darshit Shah:
On 11/18, Tim Rühsen wrote:
This patch fixes most C89 warnings for me (-std=c89 -pedantic) since these
may prevent from compiling with MSVC.
There are still some warnings ISO C forbids conversion of function pointer
to object pointer type [-Wpedantic] left over. I'll care for these the
next days. There are architectures where function pointers have a
different size from void *. That's why this warning has a meaning.
Tim
From 11baace21e1fa04a92baa395f38ebad8001e9762 Mon Sep 17 00:00:00 2001
From: Tim Ruehsen tim.rueh...@gmx.de
Date: Tue, 18 Nov 2014 22:00:48 +0100
Subject: [PATCH] Trivial fixes for C89 compliancy.
snip
diff --git a/src/gnutls.c b/src/gnutls.c
index 87d1d0b..42201e5 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -54,6 +54,10 @@ as that of the covered work. */
# include w32sock.h
#endif
+#ifdef HAVE_ALLOCA_H
+# include alloca.h
+#endif
+
#include host.h
static int
@@ -122,9 +126,10 @@ ssl_init (void)
while ((dent = readdir (dir)) != NULL)
{
struct stat st;
- char ca_file[dirlen + strlen(dent-d_name) + 2];
+ size_t ca_file_length = dirlen + strlen(dent-d_name) + 2;
+ char *ca_file = alloca(ca_file_length);
What happens when HAVE_ALLOCA_H is not defined? The above code will attempt
to call a function that does not exist and Wget will crash.
I think, we can simply malloc / free() these. Sure alloca is more
convenient, but we need a valid fallback for when it is not available.
There are systems without alloca.h header file. Thus the check.
(I just had an error report for libpsl with this issue.)
My question was. On systems where alloca.h is not available, this code has no
fallback. If alloca.h is not included, then the
char *ca_file = alloca(ca_file_length);
line will prevent the source from compiling. Should be not have something to
prevent that?
I do not know systems without alloca() function.
Wget sources already use alloca on several places.
If Wget sources are already doing this, and no one has yet complained then it
means we've probably not hit the case yet where someone tried to compile Wget on
a system without alloca.h
--
Thanking You,
Darshit Shah
pgpcK1VtKiDp4.pgp
Description: PGP signature
[Bug-wget] [PATCH] Fixing C89 warnings
This patch fixes most C89 warnings for me (-stdÈ9 -pedantic) since these may
prevent from compiling with MSVC.
There are still some warnings ISO C forbids conversion of function pointer to
object pointer type [-Wpedantic] left over. I'll care for these the next
days. There are architectures where function pointers have a different size
from void *. That's why this warning has a meaning.
Tim
From 11baace21e1fa04a92baa395f38ebad8001e9762 Mon Sep 17 00:00:00 2001
From: Tim Ruehsen tim.rueh...@gmx.de
Date: Tue, 18 Nov 2014 22:00:48 +0100
Subject: [PATCH] Trivial fixes for C89 compliancy.
---
src/ChangeLog | 6 +++
src/cookies.c | 2 +-
src/ftp-basic.c | 10 +++--
src/ftp.c | 4 +-
src/gnutls.c| 12 --
src/host.c | 2 +-
src/html-url.c | 3 +-
src/http.c | 10 +++--
src/main.c | 30 +++---
src/progress.c | 9 +++--
src/retr.c | 12 --
src/warc.c | 123 +---
12 files changed, 135 insertions(+), 88 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 4e974b0..5e94b58 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,5 +1,11 @@
2014-11-18 Tim Ruehsen tim.rueh...@gmx.de
+ * cookies.c, ftp-basic.c, ftp.c, gnutls.c, host.c,
+ html-url.c, http.c, main.c, progress.c, retr.c, warc.c:
+ Trivial fixes for C89 compliancy.
+
+2014-11-18 Tim Ruehsen tim.rueh...@gmx.de
+
* Fix warnings from clang-analyzer 3.6
gnutls.c:457:3: warning: Value stored to 'err' is never read
diff --git a/src/cookies.c b/src/cookies.c
index bf872a8..f31a4ec 100644
--- a/src/cookies.c
+++ b/src/cookies.c
@@ -518,12 +518,12 @@ check_domain_match (const char *cookie_domain, const char *host)
{
#ifdef HAVE_LIBPSL
- DEBUGP ((cdm: 1));
char *cookie_domain_lower = NULL;
char *host_lower = NULL;
const psl_ctx_t *psl;
int is_acceptable;
+ DEBUGP ((cdm: 1));
if (!(psl = psl_builtin()))
{
DEBUGP ((\nlibpsl not built with a public suffix list.
diff --git a/src/ftp-basic.c b/src/ftp-basic.c
index b6e67e2..06936ce 100644
--- a/src/ftp-basic.c
+++ b/src/ftp-basic.c
@@ -965,16 +965,18 @@ ftp_list (int csock, const char *file, bool avoid_list_a, bool avoid_list,
bool ok = false;
size_t i = 0;
- *list_a_used = false;
-
/* 2013-10-12 Andrea Urbani (matfanjol)
For more information about LIST and LIST -a please look at ftp.c,
function getftp, text __LIST_A_EXPLANATION__.
If somebody changes the following commands, please, checks also the
later i variable. */
- const char *list_commands[] = { LIST -a,
- LIST };
+ static const char *list_commands[] = {
+LIST -a,
+LIST
+ };
+
+ *list_a_used = false;
if (avoid_list_a)
{
diff --git a/src/ftp.c b/src/ftp.c
index d614a27..f02f057 100644
--- a/src/ftp.c
+++ b/src/ftp.c
@@ -2221,9 +2221,9 @@ has_insecure_name_p (const char *s)
static bool
is_invalid_entry (struct fileinfo *f)
{
- struct fileinfo *cur;
- cur = f;
+ struct fileinfo *cur = f;
char *f_name = f-name;
+
/* If the node we're currently checking has a duplicate later, we eliminate
* the current node and leave the next one intact. */
while (cur-next)
diff --git a/src/gnutls.c b/src/gnutls.c
index 87d1d0b..42201e5 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -54,6 +54,10 @@ as that of the covered work. */
# include w32sock.h
#endif
+#ifdef HAVE_ALLOCA_H
+# include alloca.h
+#endif
+
#include host.h
static int
@@ -122,9 +126,10 @@ ssl_init (void)
while ((dent = readdir (dir)) != NULL)
{
struct stat st;
- char ca_file[dirlen + strlen(dent-d_name) + 2];
+ size_t ca_file_length = dirlen + strlen(dent-d_name) + 2;
+ char *ca_file = alloca(ca_file_length);
- snprintf (ca_file, sizeof(ca_file), %s/%s, ca_directory, dent-d_name);
+ snprintf (ca_file, ca_file_length, %s/%s, ca_directory, dent-d_name);
if (stat (ca_file, st) != 0)
continue;
@@ -433,9 +438,10 @@ ssl_connect_wget (int fd, const char *hostname)
struct wgnutls_transport_context *ctx;
gnutls_session_t session;
int err,alert;
- gnutls_init (session, GNUTLS_CLIENT);
const char *str;
+ gnutls_init (session, GNUTLS_CLIENT);
+
/* We set the server name but only if it's not an IP address. */
if (! is_valid_ip_address (hostname))
{
diff --git a/src/host.c b/src/host.c
index 86bf83b..28b56b4 100644
--- a/src/host.c
+++ b/src/host.c
@@ -592,7 +592,7 @@ cache_query (const char *host)
al = hash_table_get (host_name_addresses_map, host);
if (al)
{
- DEBUGP ((Found %s in host_name_addresses_map (%p)\n, host, al));
+ DEBUGP ((Found %s in host_name_addresses_map (%p)\n, host, (void *) al));
++al-refcount;
return al;
}
diff --git a/src/html-url.c b/src/html-url.c
index 903864e..29a68d6 100644
--- a/src/html-url.c
+++
Re: [Bug-wget] [PATCH] Fixing C89 warnings
I think we use std=gnu89 or std=gnu99. I've been using these and get no warnings on clang anymore. What configure command and CFLAGS are you using? I'd like to reproduce these and try to fix them. Thanking You, Darshit Shah Sent from mobile device. Please excuse my brevity On 19-Nov-2014 2:35 am, Tim Rühsen tim.rueh...@gmx.de wrote: This patch fixes most C89 warnings for me (-std=c89 -pedantic) since these may prevent from compiling with MSVC. There are still some warnings ISO C forbids conversion of function pointer to object pointer type [-Wpedantic] left over. I'll care for these the next days. There are architectures where function pointers have a different size from void *. That's why this warning has a meaning. Tim
