From owner-freebsd-hackers@FreeBSD.ORG Fri May 18 23:09:19 2012 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5E34F1065678 for ; Fri, 18 May 2012 23:09:19 +0000 (UTC) (envelope-from delphij@delphij.net) Received: from anubis.delphij.net (anubis.delphij.net [64.62.153.212]) by mx1.freebsd.org (Postfix) with ESMTP id 41FD48FC0C for ; Fri, 18 May 2012 23:09:19 +0000 (UTC) Received: from delta.delphij.net (drawbridge.ixsystems.com [206.40.55.65]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by anubis.delphij.net (Postfix) with ESMTPSA id 9E30816AF4; Fri, 18 May 2012 16:09:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=delphij.net; s=anubis; t=1337382553; bh=/stOw9zT4/GH1Poe3ybj2Di1iXHmg3V6B9GOxRTAF6k=; h=Date:From:Reply-To:To:CC:Subject:References:In-Reply-To; b=S1d8TiUwC9rFyDBKuTXsxADKyk/h70eJ28OEcCZPdnwPWKhXjdK7gw3ngaU/3HGl9 Bi8DHkE3o4ZupNMKWH/zVXJlTQO1tYADINeez5otvK1feEFHnuYw+nm+3dGbiT5kCe wlAUVWH/6lAzysshF0qKUIBvH3n/QbMZqXa2MRPM= Message-ID: <4FB6D698.9030305@delphij.net> Date: Fri, 18 May 2012 16:09:12 -0700 From: Xin Li Organization: The FreeBSD Project MIME-Version: 1.0 To: Guy Helmer References: <4EE466CC-5F93-485C-8E1F-907F8049FD61@palisadesys.com> In-Reply-To: <4EE466CC-5F93-485C-8E1F-907F8049FD61@palisadesys.com> X-Enigmail-Version: 1.5pre Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: freebsd-hackers@freebsd.org, d@delphij.net Subject: Re: Review of changes for getnetgrent.c X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: d@delphij.net List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 May 2012 23:09:19 -0000 -----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); %%% The if (grp->ng_str[freepos] != NULL) here seems to be unnecessary to me because in most cases these are false, and free() does the test regardless. Also, I think freepos can be declared within this scope level. There are a few return without space between the keyword and return value. Other than these it looks fine to me. Thanks for working on this! Cheers, - -- Xin LI https://www.delphij.net/ FreeBSD - The Power to Serve! Live free or die -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (FreeBSD) iQEcBAEBCAAGBQJPttaYAAoJEG80Jeu8UPuzFEsH/i7JwIPdk15sP3E2/YpesYQu veavnS6tebylFhniKukN4GRsS5mpbs9AmnxbNUBfF7InlzcnxOeUX9mHJepxbZQX Bz8wgsvfxlrrseIyscdwm7b4XQK3dLv+VwpbQ4fqACOX1kGEQ/GsIc65JLyp2Gzo fRLkHRAO5s3FVT5f11vsy2Ry16AmQhL2Sg9+mrGqeIbhppmDCgWfoF+rmD/7fn15 0OuJNn/S3Cz3zo+ZHI9OE1W8mkMox4kznQmv6vH/hM3Gk1cY9h66UybuBWsY31dI WF5Y5WoJBuncFlDxGkaZv2jiRAqgkfWILVWKcjyejtGgVYPEWAjIgHLyyVk7H4g= =ewU+ -----END PGP SIGNATURE-----