libifconfig: Initial code available, looking for feedback

[Marie Helene Kvello-Aune]

On Mon, Apr 11, 2016 at 8:42 AM Ravi Pokala <rpokala at mac.com> wrote:

> -----Original Message-----
>
>
> >Date: Sat, 09 Apr 2016 13:59:29 +0000
> >From: Marie Helene Kvello-Aune <marieheleneka at gmail.com>
> >To: freebsd-net at freebsd.org
> >Subject: libifconfig: Initial code available, looking for feedback
> >Message-ID:
> >       <CALXRTbfxxf+bUev8sBoJEOfR41ZsLnB35i+4G_2Bp=
> j-eVVJQQ at 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 at gmail.com



> Thanks,
>
> Ravi (rpokala@)
>
> _______________________________________________
> freebsd-net at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe at freebsd.org"
>