Date: Wed, 3 Mar 2010 00:44:06 -0800 From: Garrett Cooper <yanefbsd@gmail.com> To: Xin LI <delphij@gmail.com> Cc: Maxim Sobolev <sobomax@freebsd.org>, freebsd <freebsd-hackers@freebsd.org> Subject: Re: svn commit: r204615 - head/sbin/newfs Message-ID: <7d6fde3d1003030044h4da7ed2ch1839ea769190bd2b@mail.gmail.com> In-Reply-To: <a78074951003022059w6cb69229td289046d3fcb3e32@mail.gmail.com> References: <201003030205.o2325AMY010089@svn.freebsd.org> <a78074951003021808y1afb3f28m35a55876245a9e7d@mail.gmail.com> <4B8DD54F.6060302@FreeBSD.org> <a78074951003022059w6cb69229td289046d3fcb3e32@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Mar 2, 2010 at 8:59 PM, Xin LI <delphij@gmail.com> wrote: > On Tue, Mar 2, 2010 at 7:19 PM, Maxim Sobolev <sobomax@freebsd.org> wrote= : >> Xin LI wrote: >>> >>> On Tue, Mar 2, 2010 at 6:05 PM, Maxim Sobolev <sobomax@freebsd.org> 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 <sys/types.h> #include <inttypes.h> #include <libutil.h> #include <stdint.h> int main(void) { int64_t anum; /* * SYNOPSIS #include <libutil.h> int expand_number(const char *buf, int64_t *num); */ (void) expand_number("10G", &anum); return 0; } /* Using parselength. */ #include <sys/types.h> /* * 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; }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?7d6fde3d1003030044h4da7ed2ch1839ea769190bd2b>