Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 5 May 2013 11:15:47 -0400
From: Rich Felker <dalias@...ifal.cx>
To: musl@...ts.openwall.com
Subject: Re: [patch] add ether_(aton, ntoa)

On Sun, May 05, 2013 at 09:11:12AM -0500, Strake wrote:
> >> ntoa: same size
> >> aton: mine is smaller
> >
> > This doesn't seem to match my results. I compared against the
> > following version of aton and it was half the size of yours:
> >
> > struct ether_addr *ether_aton_r (const char *x, struct ether_addr *p_a)
> > {
> > 	unsigned char *y = p_a->ether_addr_octet;
> > 	char c;
> > 	if (sscanf(x, "%2hhx:%2hhx:%2hhx:%2hhx:%2hhx:%2hhx%c",
> > y,y+1,y+2,y+3,y+4,y+5,y+6,&c)!=6)
> > 		return 0;
> > 	return p_a;
> > }
> >
> > Rich
> 
> My method:
> 
> $ cat test.c
> #include <netinet/ether.h>
> 
> int main (int argc, char *argu[]) {
> 	struct ether_addr *p_a;
> 	p_a = ether_aton (argu[1]);
> 	return (*p_a).ether_addr_octet[0];
> }
> 
> $ cc -I $MUSL/include -L $MUSL/lib -o test test.c && strip test && wc -c <test
> 
> Mine: 6200
> Yours: 15504

I see. These are two different but perfectly valid ways for looking at
size: size of the function itself versus size of the function with all
its dependencies. My approach to measuring size leads to a smaller
libc.so, whereas your approach leads to a smaller statically linked
binary when the binary does not otherwise use *scanf for anything.

This is an issue that's come up several times before when optimizing
for size, and I don't think there's one right answer for all cases.
The solution is obvious in cases where the code size without depending
on another function is minimal, but when a large function like
snprintf or sscanf is involved it's a larger trade-off.

My leaning in this case is towards making the ether_* functions as
small as possible themselves, without worrying about dependencies. My
reasoning is that, on a small system, at most one or two programs will
be using these functions anyway, and if it's Busybox (or in the
future, Toybox) then scanf is already being pulled in anyway. The main
cases where I'd lean the other way are for functions which have
usefulness in a large number of programs, some of which may be written
in light/minimalistic ways to avoid pulling in stuff like stdio.

Anyway, I welcome other comments on the issue; we don't have to make a
decision immediately but I'd like to get the ether_* stuff integrated
in one form or another soon and have it in the next release.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.