[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130420014945.GI20323@brightrain.aerifal.cx>
Date: Fri, 19 Apr 2013 21:49:45 -0400
From: Rich Felker <dalias@...ifal.cx>
To: musl@...ts.openwall.com
Subject: Re: [patch] add ether_(aton, ntoa)
On Sun, Apr 14, 2013 at 11:10:55PM -0500, Strake wrote:
> +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);
&x has the wrong type; strtoul requires char **, not const char **.
A separate temp var is required for the output, as in.
char *y;
n = strtoul (x, &y, 16);
x = y;
or similar. As far as I know the naive cast one could make to "fix"
the type mismatch is not valid; it would be an aliasing violation.
> + if (n > 0xFF) return 0; /* bad byte */
> + (*p_a).ether_addr_octet[ii] = n;
Is there a reason you're using (*p_a). rather than p_a-> ?
Also, I'm not sure if this is worth changing or not, but usually we
avoid writing to output pointers except on success. For standard
interfaces this is a conformance issue, but for these it probably
doesn't matter.
> +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]);
And again, why not p_a-> ?
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.
