On Tue, Mar 2, 2010 at 8:59 PM, Xin LI <delp...@gmail.com> wrote:
> On Tue, Mar 2, 2010 at 7:19 PM, Maxim Sobolev <sobo...@freebsd.org> wrote:
>> Xin LI wrote:
>>>
>>> On Tue, Mar 2, 2010 at 6:05 PM, Maxim Sobolev <sobo...@freebsd.org> wrote:
>>>>
>>>> Author: sobomax
>>>> Date: Wed Mar 3 02:05:09 2010
>>>> New Revision: 204615
>>>> URL: http://svn.freebsd.org/changeset/base/204615
>>>>
>>>> Log:
>>>> Teach newfs(8) to understand size modifiers for all options taking
>>>> size or size-like argument. I.e. "-s 32k" instead of "-s 32768".
>>>> Size parsing function has been shamelessly stolen from the truncate(1).
>>>> I'm sure many sysadmins out there will appreciate this small
>>>> improvement.
>>>
>>> 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 -hack...@]
>
> 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? Because 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
garrc...@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 = 0;
lsign = 1;
switch (*ls) {
case '-':
lsign = -1;
case '+':
ls++;
}
#define ASSIGN_CHK_OFLOW(x, y) if (x < y) return -1; y = x
/*
* Calculate the value of the decimal digit string, failing
* on overflow.
*/
while (isdigit(*ls)) {
oflow = length * 10 + *ls++ - '0';
ASSIGN_CHK_OFLOW(oflow, length);
}
switch (*ls) {
case 'T':
case 't':
oflow = length * 1024;
ASSIGN_CHK_OFLOW(oflow, length);
case 'G':
case 'g':
oflow = length * 1024;
ASSIGN_CHK_OFLOW(oflow, length);
case 'M':
case 'm':
oflow = length * 1024;
ASSIGN_CHK_OFLOW(oflow, length);
case 'K':
case 'k':
if (ls[1] != '\0')
return -1;
oflow = length * 1024;
ASSIGN_CHK_OFLOW(oflow, length);
case '\0':
break;
default:
return -1;
}
*sz = length * lsign;
return 0;
}
int
main(void) {
int anum;
(void) parselength("10G", &anum);
return 0;
}
_______________________________________________
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
