Date: Tue, 12 Apr 2016 13:45:39 +0000 From: Marie Helene Kvello-Aune <marieheleneka@gmail.com> To: Ravi Pokala <rpokala@mac.com>, "freebsd-net@freebsd.org" <freebsd-net@freebsd.org> Subject: Re: libifconfig: Initial code available, looking for feedback Message-ID: <CALXRTbenxK6ACHOAkkgrWE9kOD3YvfGF_d-yv53iAofcDcwm7Q@mail.gmail.com> In-Reply-To: <25165EFB-4D17-476D-86C6-D99C3E97F227@panasas.com> References: <25165EFB-4D17-476D-86C6-D99C3E97F227@panasas.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Apr 11, 2016 at 8:42 AM Ravi Pokala <rpokala@mac.com> wrote: > -----Original Message----- > > > >Date: Sat, 09 Apr 2016 13:59:29 +0000 > >From: Marie Helene Kvello-Aune <marieheleneka@gmail.com> > >To: freebsd-net@freebsd.org > >Subject: libifconfig: Initial code available, looking for feedback > >Message-ID: > > <CALXRTbfxxf+bUev8sBoJEOfR41ZsLnB35i+4G_2Bp= > j-eVVJQQ@mail.gmail.com> > >Content-Type: text/plain; charset=UTF-8 > > > >Hey! > > > >Please see previous thread[1] for context on what libifconfig is. > > > >I've just pushed an initial version of libifconfig to the github > >repository[2]. I would appreciate feedback, in particular on the API > design > >and usage, and especially on how it communicates error state to the > >application. > > Hi, > > I'm definitely not a networking person, but I have some comments: > > Thanks for the feedback! I've made note of all of it, even though I'm not commenting on all of it. :) > libifconfig.h > > #pragma once - I'm not sure if that's supported in every compiler used > in the base system; I'd stick with a traditional guard macro. > > I did research this some, and came to the conclusion it's supported. But I'll take another look at it. > libifconfig_ioctlwrap() / libifconfig_ioctlwrap_caddr() - it's not > clear why you're using one versus the other. > > I'm basically reusing ifconfig code where I can, and the ifconfig code has some places where it casts to caddr_t and some places it doesn't. I haven't researched whether casting is necessary yet, and therefore I'm mimicking the original code as closely as I can, to be on the safe side. If it turns out it's unnecessary, it should be a simple task to clean it up. :) > libifconfig_get_description() - the case of the ioctl failing is not > handled. > Good catch. This code was fine in ifconfig because the later break statement ends the loop, but this is not sufficient for libifconfig. :) libifconfig_socketcache.c > > libifconfig_socket() - similar to libifconfig_ioctlwrap_ret(), the > value of "errno" is lost. > > It looks like I'll have to restructure the errstate struct a bit and make it clear it's a stored errno. But at that point it may be pointless to store it there, as 'errno' is thread safe, and the calling application can access that directly. [1] Issue 1: https://github.com/Savagedlight/libifconfig/issues/1 Thanks! -- Marie Helene Kvello-Aune marieheleneka@gmail.com > Thanks, > > Ravi (rpokala@) > > _______________________________________________ > freebsd-net@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/freebsd-net > To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org" >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CALXRTbenxK6ACHOAkkgrWE9kOD3YvfGF_d-yv53iAofcDcwm7Q>