On Tue, 5 Jul 2016 10:53:38 -0500 Eli Cohen <e...@mellanox.com> wrote:
> > +static int extract_guid(__u64 *guid, char *arg) > +{ > + __u64 ret; > + int g[8]; > + int err; > + > + err = sscanf(arg, "%02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x", > + g, g + 1, g + 2, g + 3, g + 4, g + 5, g + 6, g + 7); > + if (err != 8) > + return -1; > + > + ret = ((__u64)(g[0]) << 56) | > + ((__u64)(g[1]) << 48) | > + ((__u64)(g[2]) << 40) | > + ((__u64)(g[3]) << 32) | > + ((__u64)(g[4]) << 24) | > + ((__u64)(g[5]) << 16) | > + ((__u64)(g[6]) << 8) | > + ((__u64)(g[7])); > + *guid = ret; > + > + return 0; > +} I would like several things changed here. 1. put this in generic (ie lib/utils.c) so that other places can use it. And rename it match other arg parsing code (ie get_guid) 2. need range checking for each piece the string, and each hex piece must be unsigned int suprised gcc format checks didn't bust you on this. why not %hhx as format specifier 3. arg should be const char * 4. local variable err is really unnecessary 5. local variable ret is unnecessary, you could just assign to *guid