From owner-svn-src-projects@freebsd.org Wed Aug 12 20:14:42 2015 Return-Path: Delivered-To: svn-src-projects@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 76FD49AB688 for ; Wed, 12 Aug 2015 20:14:42 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (relay02.stack.nl [IPv6:2001:610:1108:5010::104]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client CN "mailhost.stack.nl", Issuer "CA Cert Signing Authority" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 3EE80E30; Wed, 12 Aug 2015 20:14:42 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131]) by mx1.stack.nl (Postfix) with ESMTP id DD364359305; Wed, 12 Aug 2015 22:14:38 +0200 (CEST) Received: by snail.stack.nl (Postfix, from userid 1677) id ACCDA28494; Wed, 12 Aug 2015 22:14:38 +0200 (CEST) Date: Wed, 12 Aug 2015 22:14:38 +0200 From: Jilles Tjoelker To: Bruce Evans Cc: Baptiste Daroussin , src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: Re: svn commit: r286521 - projects/collation/lib/libc/locale Message-ID: <20150812201438.GA26529@stack.nl> References: <201508091150.t79Boo3v096088@repo.freebsd.org> <20150809223647.O2415@besplex.bde.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150809223647.O2415@besplex.bde.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 12 Aug 2015 20:14:42 -0000 On Sun, Aug 09, 2015 at 10:54:15PM +1000, Bruce Evans wrote: > On Sun, 9 Aug 2015, Baptiste Daroussin wrote: > > Log: > > Use asprintf/free instead of snprintf > Why? It takes 3 times as much code, and immediately gave you a memory > leak when you wrote only twice as much. You fixed the memory leak in the > next commit, but it might not always be so easy to see. > > Modified: projects/collation/lib/libc/locale/collate.c > > ============================================================================== > > --- projects/collation/lib/libc/locale/collate.c Sun Aug 9 11:47:01 2015 (r286520) > > +++ projects/collation/lib/libc/locale/collate.c Sun Aug 9 11:50:50 2015 (r286521) > > @@ -107,7 +107,7 @@ int > > __collate_load_tables_l(const char *encoding, struct xlocale_collate *table) > > { > > int i, chains, z; > > - char buf[PATH_MAX]; > > + char *buf; > POSIX_ME_HARDER code would use {PATH_MAX} = sysconf(__PATH_MAX) and error > handling for sysconf() and then for malloc()ing {PATH_MAX} bytes. This > would take 10 times as much code, except it could use a VLA with no > error checking for the allocation starting with C99. The asprintf() > method would be better then. No, POSIX_ME_HARDER code would use dynamic allocation (but without asprintf() since that's not in POSIX). This is because {PATH_MAX} may be indeterminate (pathconf() returns -1 but does not set errno). Note that some usages of certain functions are invalid when {PATH_MAX} is indeterminate, for example realpath() with a non-NULL resolved_path parameter. It seems uncommon to have a fixed {PATH_MAX} but not define it as a compile-time constant. An indeterminate {PATH_MAX} is used in GNU Hurd and its developers occasionally patch software to make it cope with that. I think it is fine to use dynamic allocation here. -- Jilles Tjoelker