From owner-svn-src-all@FreeBSD.ORG Sun Feb 15 08:40:06 2015 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 6CC40852; Sun, 15 Feb 2015 08:40:06 +0000 (UTC) Received: from mail108.syd.optusnet.com.au (mail108.syd.optusnet.com.au [211.29.132.59]) by mx1.freebsd.org (Postfix) with ESMTP id 2ABCABA3; Sun, 15 Feb 2015 08:40:05 +0000 (UTC) Received: from c211-30-166-197.carlnfd1.nsw.optusnet.com.au (c211-30-166-197.carlnfd1.nsw.optusnet.com.au [211.30.166.197]) by mail108.syd.optusnet.com.au (Postfix) with ESMTPS id 480C91A1D02; Sun, 15 Feb 2015 19:40:02 +1100 (AEDT) Date: Sun, 15 Feb 2015 19:40:01 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: John-Mark Gurney Subject: Re: svn commit: r278737 - head/usr.sbin/flowctl In-Reply-To: <20150215074419.GB1953@funkthat.com> Message-ID: <20150215185431.M1467@besplex.bde.org> References: <201502132357.t1DNvKda075915@svn.freebsd.org> <20150214193210.N945@besplex.bde.org> <20150214181508.GL15484@FreeBSD.org> <1423938828.80968.148.camel@freebsd.org> <54DFA7CC.20305@FreeBSD.org> <20150215162553.L977@besplex.bde.org> <20150215074419.GB1953@funkthat.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=baJSDo/B c=1 sm=1 tr=0 a=KA6XNC2GZCFrdESI5ZmdjQ==:117 a=PO7r1zJSAAAA:8 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=qqYFbTANiK4VTEzh2kIA:9 a=CjuIK1q_8ugA:10 Cc: src-committers@freebsd.org, Ian Lepore , svn-src-all@freebsd.org, Pedro Giffuni , Gleb Smirnoff , Bruce Evans , svn-src-head@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 15 Feb 2015 08:40:06 -0000 On Sat, 14 Feb 2015, John-Mark Gurney wrote: > Bruce Evans wrote this message on Sun, Feb 15, 2015 at 17:53 +1100: >> On Sat, 14 Feb 2015, Pedro Giffuni wrote: >> >>> On 02/14/15 13:33, Ian Lepore wrote: >>>> On Sat, 2015-02-14 at 21:15 +0300, Gleb Smirnoff wrote: >>>>> On Sat, Feb 14, 2015 at 08:46:58PM +1100, Bruce Evans wrote: >>>>> B> Using VLAs and also the C99 feature of declarations anwhere, and >>>>> extensions >>>>> B> like __aligned(), we can almost implement a full alloca() using the >>>>> fixed >>>>> B> version of this change: >>>>> B> >>>>> B> /* >>>>> B> * XXX need extended statement-expression so that __buf doesn't go out >>>>> B> * of scope after the right brace. >>>>> B> */ >>>>> B> #define my_alloca(n) __extension__ ({ >>>>> B> /* XXX need unique name. */ \ >>>>> B> char __buf[__roundup2((n), MUMBLE)] __aligned(MUMBLE); \ >>>>> B> \ >>>>> B> (void *)__buf; \ >>>>> B> }) >>>>> >>>>> I like this idea. But would this exact code work? The life of >>>>> __buf is limited by the code block, and we exit the block >>>>> immediately. Wouldn't the allocation be overwritten if we >>>>> enter any function or block later? > > Could this just be changed to something like: > struct ng_mesg ng_mesg[(SORCVBUF_SIZE + sizeof(struct ng_mesg) - 1) / > sizeof(struct ng_mesg)]; > > It might allocate a few extra bytes, but no more than 55, and gets > alignment correct w/o lots of other hacks... I already gave this version, with (SORCVBUF_SIZE + sizeof(struct ng_mesg) - 1) / sizeof(struct ng_mesg) spelled as howmany(SORCVBUF_SIZE, sizeof(*ng_mesg)). Even with the better spelling, it is less clear than using alloca(). Using malloc() would be equally clear to using alloca(), except it would take even more code unless all of error checking, error handling and deallocation were omitted. Presumably the unnecessary complications for that are the reason why malloc() wasn't used originally. However, it may be considered a style bug to use alloca() when almost everything else laboriously uses malloc(). "alloca(" occurs in 453 lines in /usr/src/. 110 not counting cddl, contrib, crypto or sys. 35 of the 110 are in jail; 10 in libjail; 12 in ctm; 7 in ppp; 8 in pppoed. That leaves 38. The only applications in these 38 are swapon, rpcbind, jls, flowctl and vidcontrol. Some of these could probably use VLAs. E.g., vidcontrol uses alloca() just twice, for char buffers. It has laborious error checking and handling after each. But the error checking is essentially useless since the libc alloca() that can return NULL is never used. More critical/security-related applications like jail of course don't have the error checking or handling. Bruce