|
Message-ID: <CAL3m8eCdz_eUGptYsn5NDPho0PQ_=wH+F4aY9eSmgsPj+vS00w@mail.gmail.com> Date: Sun, 14 Apr 2013 23:10:55 -0500 From: Strake <strake888@...il.com> To: musl@...ts.openwall.com Subject: Re: [patch] add ether_(aton, ntoa) On 14/04/2013, Rich Felker <dalias@...ifal.cx> wrote: > I would put the static object inside the function that uses it rather > than outside, but this is purely a stylistic consideration. Yeah, I just like to define closure-bound free variables without the closure scope itself, and automatics within. > Also, it > _might_ be better to separate the functions with static storage out so > that programs using the _r interfaces don't pull in extra .bss, but > it's a tradeoff -- if we do this for every ugly nonstandard function, > we just end up making the .a file a lot larger and making build time a > lot slower. So I think I'm okay with the way you did it; we might even > want to go the other direction and put all 4 functions in one file. I would've done, but I wished to match how inet_* are organized. >> +char *ether_ntoa_r (const struct ether_addr *p_a, char *x) { >> + char *y; >> + y = x; >> + for (int ii = 0; ii < 6; ii++) { >> + x += sprintf (x, ii == 0 ? "%0hhX" : ":%0hhX", >> (*p_a).ether_addr_octet[ii]); > > The hh is unnecessary here. The argument has type int (due to default > promotions) so just "%X" is fine. Also, as far as I can tell, the 0 > was not doing anything since no width was specified. If your intent is > to always have 2 hex digits, "%.2X" is the right format to use. Thanks. New patch follows message. > Could you compare and see if it generates smaller code if we use a > single snprintf/sscanf to implement these functions rather than a > loop? I'm not sure which is better, but since they're not widely used, > my main interest is keeping them small. ntoa: same size aton: mine is smaller > Also, how are these functions > documented to handle malformed input, and are you matching the > documentation on that? Linux docs say that ether_aton return 0 on malformed input, as mine does. ether_ntoa is infallible. Cheers, Strake >From ad5ab07c6ec15b8b98717750888947634c2fa039 Mon Sep 17 00:00:00 2001 From: Strake <strake888@...il.com> Date: Sun, 14 Apr 2013 12:09:30 -0500 Subject: [PATCH] add ether_(aton, ntoa) --- include/netinet/ether.h | 14 ++++++++++++++ src/network/ether_aton.c | 23 +++++++++++++++++++++++ src/network/ether_ntoa.c | 17 +++++++++++++++++ 3 files changed, 54 insertions(+) create mode 100644 include/netinet/ether.h create mode 100644 src/network/ether_aton.c create mode 100644 src/network/ether_ntoa.c diff --git a/include/netinet/ether.h b/include/netinet/ether.h new file mode 100644 index 0000000..c5179d5 --- /dev/null +++ b/include/netinet/ether.h @@ -0,0 +1,14 @@ +#ifndef _NETINET_ETHER_H +#define _NETINET_ETHER_H + +#include <netinet/if_ether.h> + +char *ether_ntoa (const struct ether_addr *); + +struct ether_addr *ether_aton (const char *); + +char *ether_ntoa_r (const struct ether_addr *, char *); + +struct ether_addr *ether_aton_r (const char *, struct ether_addr *); + +#endif diff --git a/src/network/ether_aton.c b/src/network/ether_aton.c new file mode 100644 index 0000000..4270872 --- /dev/null +++ b/src/network/ether_aton.c @@ -0,0 +1,23 @@ +#include <stdlib.h> +#include <netinet/ether.h> + +static struct ether_addr a; + +struct ether_addr *ether_aton_r (const char *x, struct ether_addr *p_a) { + for (int ii = 0; ii < 6; ii++) { + unsigned long int n; + if (ii != 0) { + if (x[0] != ':') return 0; /* bad format */ + else x++; + } + n = strtoul (x, &x, 16); + if (n > 0xFF) return 0; /* bad byte */ + (*p_a).ether_addr_octet[ii] = n; + } + if (x[0] != 0) return 0; /* bad format */ + return p_a; +} + +struct ether_addr *ether_aton (const char *x) { + return ether_aton_r (x, &a); +} diff --git a/src/network/ether_ntoa.c b/src/network/ether_ntoa.c new file mode 100644 index 0000000..468ac48 --- /dev/null +++ b/src/network/ether_ntoa.c @@ -0,0 +1,17 @@ +#include <stdio.h> +#include <netinet/ether.h> + +static char x[18]; + +char *ether_ntoa_r (const struct ether_addr *p_a, char *x) { + char *y; + y = x; + for (int ii = 0; ii < 6; ii++) { + x += sprintf (x, ii == 0 ? "%.2X" : ":%.2X", (*p_a).ether_addr_octet[ii]); + } + return y; +} + +char *ether_ntoa (const struct ether_addr *p_a) { + return ether_ntoa_r (p_a, x); +}
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.