Date: Mon, 21 May 2012 16:10:51 -0500 From: Guy Helmer <ghelmer@palisadesys.com> To: d@delphij.net Cc: freebsd-hackers@freebsd.org Subject: Re: Review of changes for getnetgrent.c Message-ID: <0C6268C5-3E3C-40F2-A8C6-23615D05893D@palisadesys.com> In-Reply-To: <4FBA978F.2030908@delphij.net> References: <4EE466CC-5F93-485C-8E1F-907F8049FD61@palisadesys.com> <4FB6D698.9030305@delphij.net> <AE6E3D09-1413-4B5E-8A79-1145179F8D62@palisadesys.com> <4FBA978F.2030908@delphij.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On May 21, 2012, at 2:29 PM, Xin Li wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> On 05/21/12 12:02, Guy Helmer wrote:
>>
>> On May 18, 2012, at 6:09 PM, Xin Li wrote:
>>
>>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
>>>
>>> On 05/18/12 14:58, Guy Helmer wrote:
>>>> To close PR bin/83340, I have this change worked up to resolve
>>>> memory allocation failure handling and avoid creating bad
>>>> entries in the grp list due to memory allocation failures while
>>>> building a new entry.
>>>>
>>>> Before committing, I wanted to run it past others to see if
>>>> there were any problems with it.
>>>
>>> %%% @@ -477,6 +475,13 @@ if (len > 0) { grp->ng_str[strpos] =
>>> (char *) malloc(len + 1); + if (grp->ng_str[strpos] == NULL)
>>> { + for (freepos = 0; freepos < strpos; freepos++) +
>>> if (grp->ng_str[freepos] != NULL) +
>>> free(grp->ng_str[freepos]); + free(grp); +
>>> return(1); + } bcopy(spos, grp->ng_str[strpos], len + 1);
>>> %%%
>>
>> Like this?
>>
>> if (len > 0) { grp->ng_str[strpos] = (char *) malloc(len + 1); +
>> if (grp->ng_str[strpos] == NULL) { + int freepos; + for
>> (freepos = 0; freepos < strpos; freepos++) +
>> free(grp->ng_str[freepos]); + free(grp); + return(1); +
>> } bcopy(spos, grp->ng_str[strpos], len + 1); }
>>>
>>> There are a few return without space between the keyword and
>>> return value.
>>
>> Do you recommend I fix all those instances in the file, or just the
>> instances in this patch?
>
> I'd recommend fixing them all (note that you could run into a bigger
> commit as the switch() is not style(9) conformant at this time) and we
> normally do it in two different commits (one style, and another
> functional) when possible.
>
OK, thank you.
Guy
--------
This message has been scanned by ComplianceSafe, powered by Palisade's PacketSure.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?0C6268C5-3E3C-40F2-A8C6-23615D05893D>