bug-wget
[Top][All Lists]
Advanced

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

Re: [Bug-wget] [PATCH] Fix some clang-analyzer warnings


From: Darshit Shah
Subject: Re: [Bug-wget] [PATCH] Fix some clang-analyzer warnings
Date: Wed, 19 Nov 2014 13:28:29 +0530
User-agent: Mutt/1.5.23 (2014-03-12)

On 11/18, Giuseppe Scrivano wrote:
Tim Rühsen <address@hidden> writes:

Fixes these warnings:

        gnutls.c:457:3: warning: Value stored to 'err' is never read
          err = 0;

        http-ntlm.c:477:5: warning: Value stored to 'size' is never read
          size = (size_t) snprintf (ntlmbuf, sizeof(ntlmbuf),

        http.c:1479:3: warning: Attempt to free released memory
          xfree_null (hs->error);

The last one *might* result in a crash under special circumstances.


I think we should just have one xfree() macro instead of two (xfree and
xfree_null, some parts of the code even use free() directly).

IMHO, a free'd pointer should always be set to NULL afterwards (as a good
programming convention). I suggest the following macro

#define xfree(a) do { if (a) { free ((void *) (a)); a = NULL; } } while (0)

I think you can skip the check, gnulib ensures that free(NULL) is a
no-op.

I just hit this issue. (Lucky timing, I guess)

It seems to me that you and Tim agreed upon the same thing. We should have only a xfree() macro since gnulib will ensure that a NULL free(NULL) is a no-op.

However, the xfree() macro should explicitly set the pointer to NULL. That does seem like sound programming practice. And I just hit a snag with valgrind where it was complaining about a double free. But setting the pointer to NULL explicitly silenced it.

--
Thanking You,
Darshit Shah

Attachment: pgpmMY3O8oFmF.pgp
Description: PGP signature


reply via email to

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