bug-wget
[Top][All Lists]
Advanced

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

Re: [Bug-wget] [PATCH] Fixing C89 warnings


From: Darshit Shah
Subject: Re: [Bug-wget] [PATCH] Fixing C89 warnings
Date: Thu, 20 Nov 2014 14:08:27 +0530
User-agent: Mutt/1.5.23 (2014-03-12)

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 <address@hidden>
>> >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

Attachment: pgpGNbzUAWP5R.pgp
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]