Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 26 Feb 2017 21:51:47 -0500
From:      Pedro Giffuni <pfg@FreeBSD.org>
To:        Cy Schubert <Cy.Schubert@komquats.com>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r314322 - head/lib/librss
Message-ID:  <6a8a34b7-e6ff-b8e2-44a6-7e3cc08e6e3d@FreeBSD.org>
In-Reply-To: <201702270237.v1R2bSsC004156@slippy.cwsent.com>
References:  <201702270237.v1R2bSsC004156@slippy.cwsent.com>

next in thread | previous in thread | raw e-mail | index | archive | help


On 2/26/2017 9:37 PM, Cy Schubert wrote:
> In message <404d743b-735b-0605-5ab5-ccb95ce24ad8@FreeBSD.org>, Pedro
> Giffuni wr
> ites:
>> This is a multi-part message in MIME format.
>> --------------C3FC59CAC7D072A09BF70AAF
>> Content-Type: text/plain; charset=windows-1252; format=flowed
>> Content-Transfer-Encoding: 7bit
>>
>> Hello;
>>
>> On 2/26/2017 8:51 PM, Cy Schubert wrote:
>>> In message <201702270010.v1R0A1wm074123@repo.freebsd.org>, "Pedro F.
>>> Giffuni" w
>>> rites:
>>>> Author: pfg
>>>> Date: Mon Feb 27 00:10:00 2017
>>>> New Revision: 314322
>>>> URL: https://svnweb.freebsd.org/changeset/base/314322
>>>>
>>>> Log:
>>>>     librss: simplify some NULL checks.
>>>>     
>>>>     MFC after:	1 week
>>>>
>>>> Modified:
>>>>     head/lib/librss/librss.c
>>>>
>>>> Modified: head/lib/librss/librss.c
>>>> ==========================================================================
>> ===
>>>> =
>>>> --- head/lib/librss/librss.c	Sun Feb 26 22:17:06 2017	(r31432
>> 1)
>>>> +++ head/lib/librss/librss.c	Mon Feb 27 00:10:00 2017	(r31432
>> 2)
>>>> @@ -244,10 +244,10 @@ rss_config_get(void)
>>>>    	return (rc);
>>>>    
>>>>    error:
>>>> -	if ((rc != NULL) && rc->rss_bucket_map)
>>>> +	if (rc != NULL) {
>>>>    		free(rc->rss_bucket_map);
>>> What happens if rc is not NULL and rc->rss_bucket_map is NULL?
>> According the free(3): " If /ptr/ is *NULL*, no action occurs."
>>
> Good point.
>
> Then why even test for RC being NULL?

If rc is NULL, there is a chance that rc->rss_bucket_map is random 
garbage and you might be "double freeing" something.

> The only reason I can think of doing any test for NULL before free(3) is
> that if the likelihood of *ptr being NULL is greater than the likelihood of
> *ptr not being NULL then you save running the extra instructions to make
> that determination in free(), e.g. pushes, call, return, pops.
>

The check for rc NULL is necessary, the only optimization is that having 
done the check we already know if we need to free(rc), so while we could 
leave the second free() outside the branch it is better to leave it 
within the "if" .

Pedro.




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a8a34b7-e6ff-b8e2-44a6-7e3cc08e6e3d>