From owner-freebsd-hackers@FreeBSD.ORG Wed Mar 3 08:48:15 2010 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 5512E1065689 for ; Wed, 3 Mar 2010 08:48:15 +0000 (UTC) (envelope-from yanefbsd@gmail.com) Received: from mail-pw0-f54.google.com (mail-pw0-f54.google.com [209.85.160.54]) by mx1.freebsd.org (Postfix) with ESMTP id B4F8C8FC79 for ; Wed, 3 Mar 2010 08:44:14 +0000 (UTC) Received: by pwj1 with SMTP id 1so811962pwj.13 for ; Wed, 03 Mar 2010 00:44:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=l9T6QuXx0Y2JndaSU7zf4/XLQVC2f0uUSXbbnV3jft4=; b=xLdmjr5hRT1U9k8kIITyaGWlxLaTc0+dsF3SVmuZEo9/EYzLGD17oiuSBYNa95zR4t NRd660CDJ0o4L2ETPIpu9o29dhFoCPe2Us87uuBD1yPwHDLwYj8yl0LM5ifXsLva6ED6 iXani6/hIC05pMJ0EAkDBicD0DXgTMGjkYqwk= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=BvKWQ3bTdVXUJYVn0C62LL2PzWOVzGlm/xf7FF+1oOb3xKkzkzXF7B8EnMIwTgWRem YeShaHBOKoJc+FdFWwLOtfOeKCL3XYSF5v7DHO/AgzHf7DtsFWgTZkAm8BxB/5gc3CBB e887ajEjZpvV98RBHr1wpdax1IhfQtGGwJW+M= MIME-Version: 1.0 Received: by 10.142.249.24 with SMTP id w24mr980709wfh.175.1267605846544; Wed, 03 Mar 2010 00:44:06 -0800 (PST) In-Reply-To: References: <201003030205.o2325AMY010089@svn.freebsd.org> <4B8DD54F.6060302@FreeBSD.org> Date: Wed, 3 Mar 2010 00:44:06 -0800 Message-ID: <7d6fde3d1003030044h4da7ed2ch1839ea769190bd2b@mail.gmail.com> From: Garrett Cooper To: Xin LI Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: Maxim Sobolev , freebsd Subject: Re: svn commit: r204615 - head/sbin/newfs X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Mar 2010 08:48:16 -0000 On Tue, Mar 2, 2010 at 8:59 PM, Xin LI wrote: > On Tue, Mar 2, 2010 at 7:19 PM, Maxim Sobolev wrote= : >> Xin LI wrote: >>> >>> On Tue, Mar 2, 2010 at 6:05 PM, Maxim Sobolev wro= te: >>>> >>>> Author: sobomax >>>> Date: Wed Mar =A03 02:05:09 2010 >>>> New Revision: 204615 >>>> URL: http://svn.freebsd.org/changeset/base/204615 >>>> >>>> Log: >>>> =A0Teach newfs(8) to understand size modifiers for all options taking >>>> =A0size or size-like argument. I.e. "-s 32k" instead of "-s 32768". >>>> =A0Size parsing function has been shamelessly stolen from the truncate= (1). >>>> =A0I'm sure many sysadmins out there will appreciate this small >>>> =A0improvement. >>> >>> Bikeshed: why not expand_number()? >> >> I did not know that function existed, but even if I did, I am really not >> sure if adding dependency on external library just to save 200 bytes of = code >> worth it. Considering that newfs(8) is often embedded into various >> space-tight/custom things, adding dependency could cause more harm than >> good. In any case, I do not feel strongly about that, so I can change it= to >> use libutil if people feel like it. > > [Moved from svn-all@ to -hackers@] > > I'd prefer depending on libutil since it's installed as a /lib library > which is usually available, as libutil is not something easily > avoidable. > > By the way I'm curious why these (humanize and friends) are not > available as libc function? =A0Because they are not part of POSIX > perhaps? Maxim, Xin Li has a point. I ran some tests and the ad hoc parsing function eats up more memory than expand_number(3) [*]: $ uname -a FreeBSD garrcoop-fbsd.cisco.com 8.0-STABLE FreeBSD 8.0-STABLE #2: Wed Feb 3 16:57:07 PST 2010 garrcoop@garrcoop-fbsd.cisco.com:/usr/obj/usr/src/sys/LAPPY_X86 i386 $ gcc -o test_expand_number test_expand_number.c -lutil; strip test_expand_number; stat -f '%z %b' test_expand_number 3240 8 $ gcc -o test_non-expand_number test_non-expand_number.c; strip test_non-expand_number; stat -f '%z %b' test_non-expand_number 3756 8 This of course is dynamically linked, not statically linked; the statically linked size is of course ridiculous: $ gcc -static -o test_expand_number test_expand_number.c -lutil; strip test_expand_number; stat -f '%z %b' test_expand_number 184744 364 But just to be fair the non-expand number version is bloody near the same (only 540 bytes smaller on disk)... $ gcc -static -o test_non-expand_number test_non-expand_number.c; strip test_non-expand_number; stat -f '%z %b' test_non-expand_number 184204 360 Given that expand_number(3) is more established and tested, and supports more prefixes / 64-bit numbers, doesn't it make more sense to use expand_number(3) given the above evidence? Thanks, -Garrett [*] Notes: 1. All of these numbers were obtained with non-optimized CFLAGS (-O0, no -march values, etc). 2. All of these numbers were obtained after stripping the binaries as shown above. /* Using expand_number(3) */ #include #include #include #include int main(void) { int64_t anum; /* * SYNOPSIS #include int expand_number(const char *buf, int64_t *num); */ (void) expand_number("10G", &anum); return 0; } /* Using parselength. */ #include /* * Return the numeric value of a string given in the form [+-][0-9]+[GMKT] * or -1 on format error or overflow. */ static int parselength(const char *ls, int *sz) { off_t length, oflow; int lsign; length =3D 0; lsign =3D 1; switch (*ls) { case '-': lsign =3D -1; case '+': ls++; } #define ASSIGN_CHK_OFLOW(x, y) if (x < y) return -1; y =3D x /* * Calculate the value of the decimal digit string, failing * on overflow. */ while (isdigit(*ls)) { oflow =3D length * 10 + *ls++ - '0'; ASSIGN_CHK_OFLOW(oflow, length); } switch (*ls) { case 'T': case 't': oflow =3D length * 1024; ASSIGN_CHK_OFLOW(oflow, length); case 'G': case 'g': oflow =3D length * 1024; ASSIGN_CHK_OFLOW(oflow, length); case 'M': case 'm': oflow =3D length * 1024; ASSIGN_CHK_OFLOW(oflow, length); case 'K': case 'k': if (ls[1] !=3D '\0') return -1; oflow =3D length * 1024; ASSIGN_CHK_OFLOW(oflow, length); case '\0': break; default: return -1; } *sz =3D length * lsign; return 0; } int main(void) { int anum; (void) parselength("10G", &anum); return 0; }