Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 20 Oct 2012 20:41:37 -0400
From: Rich Felker <dalias@...ifal.cx>
To: musl@...ts.openwall.com
Subject: Re: [PATCH 3/4] Import BSD functions defined in
 <netinet/ether.h> from NetBSD

On Sun, Oct 21, 2012 at 02:42:52AM +0200, John Spencer wrote:
> On 10/20/2012 10:15 PM, Abdoulaye Walsimou Gaye wrote:
> >+/*
> >+ * ether_aton():
> >+ * This function converts an ASCII string of the same form and to a structure
> >+ * containing the 6 octets of the address.  It returns a pointer to a
> >+ * static structure that is reused for each call.
> >+ */
> >+struct ether_addr *ether_aton(const char *s)
> >+{
> >+	static struct ether_addr n;
> >+	unsigned int i[6];
> >+
> >+	assert(s != NULL);
> >+
> >+	if (sscanf(s, " %x:%x:%x:%x:%x:%x ",&i[0],&i[1],
> >+	&i[2],&i[3],&i[4],&i[5]) == 6) {
> >+		n.ether_addr_octet[0] = (unsigned char)i[0];
> >+		n.ether_addr_octet[1] = (unsigned char)i[1];
> >+		n.ether_addr_octet[2] = (unsigned char)i[2];
> >+		n.ether_addr_octet[3] = (unsigned char)i[3];
> >+		n.ether_addr_octet[4] = (unsigned char)i[4];
> >+		n.ether_addr_octet[5] = (unsigned char)i[5];
> >+		return&n;
> >+	}
> >+	return NULL;
> >+}
> 
> why do you duplicate the code and not use simply the _r functions
> from [4/4] with the static buffer ?

The 4/4 patch removes this code and replaces it with a call to the _r
version.

> btw the usage of sscanf is both bloated and slow.

I disagree. If these functions are rarely used, delegating the work to
sscanf avoids adding bloated parsing code. Yes it means applications
that use these functions will pull in the scanf framework, but...

> anyway, i doubt it makes sense to add this crap; i never needed it
> for sabotage which is almost feature complete and compiles 250
> packages.

...as you just said, such applications are very rare.

Anyway, I don't think it's constructive to call it "crap". Presumably
there are some programs, most likely network configuration tools,
packet sniffers, etc. that use this code to format ethernet addresses
for printing, which is a reasonable need. I do question whether we
need to support /etc/ethers (I've never heard of anybody using it),
but I don't think immediately classifying attempts at contribution as
"crap" is conducive to building a good community.

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.