Looks nice, thanks :)
> -----Original Message-----
> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> Sent: Donnerstag, 12. Mai 2016 17:36
> To: Langer, Christoph <christoph.lan...@sap.com>
> Cc: net-dev@openjdk.java.net
> Subject: Re: RFR(S): 8156521: Minor Fixes and cleanups in NetworkInterface.c
>
> Pushed, with a few additional stylistic edits.
> http://hg.openjdk.java.net/jdk9/dev/jdk/rev/fe3e1508653e
>
> On 12 May 2016, at 12:00, Langer, Christoph <christoph.lan...@sap.com>
> wrote:
>
> > Thanks Chris.
> >
> > When you finally wrap it up, maybe you want to have a look at one thing
> which I didn't dare to clean up. In line 28 you find:
> >
> > #if defined(_ALLBSD_SOURCE) && defined(__OpenBSD__)
> > #include <sys/types.h>
> > #endif
>
> I omitted this from the current set of cleanups. It can be done later,
> if needed.
>
> -Chris.
>
> > But <sys/types.h> will also be included unconditionally for all platforms
> > in line
> 34. So maybe this part for OpenBSD could be removed.
> > It would change the include order on OpenBSD, of course. As I don't have an
> OpenBSD build environment I decided not to touch this but you could remove it
> if you have more confidence...
> >
> > Best regards
> > Christoph
> >
> >
> >> -----Original Message-----
> >> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> >> Sent: Donnerstag, 12. Mai 2016 12:43
> >> To: Langer, Christoph <christoph.lan...@sap.com>
> >> Cc: Alan Bateman <alan.bate...@oracle.com>; Dmitry Samersoff
> >> <dmitry.samers...@oracle.com>; Mark Sheppard
> >> <mark.shepp...@oracle.com>; net-dev@openjdk.java.net
> >> Subject: Re: RFR(S): 8156521: Minor Fixes and cleanups in
> NetworkInterface.c
> >>
> >> On 11 May 2016, at 22:27, Langer, Christoph <christoph.lan...@sap.com>
> >> wrote:
> >>
> >>> Hi again,
> >>>
> >>> I'm done with the new version:
> >> http://cr.openjdk.java.net/~clanger/webrevs/8156521.1/
> >>>
> >>> I now tried to break lines that are too long and also fixed some other
> >>> space
> >> and indentation issues.
> >>>
> >>> To incorporate Mark's suggestions regarding plen in enumIPv6Interfaces, I
> >> consistently renamed it to "prefix" in all places, also in the prefix
> >> function
> for
> >> BSD. I also reverted back to scanf as int but then cast prefix to short on
> >> all
> >> relevant calls to addif.
> >>>
> >>> I verified the build on Linux, AIX, Solaris and MAC and ran a basic
> >> NetworkInterface test to list interfaces and print addresses and
> >> attributes. I
> >> think I'm done.
> >>>
> >>> Chris, would you be so kind to push it when you consider it reviewed? I'm
> still
> >> only author.
> >>
> >> I will take a final pass over the updated webrev, do some testing, and then
> >> push it for you.
> >>
> >> -Chris.
> >>
> >>> Thanks and best regards
> >>> Christoph
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> >>>> Sent: Mittwoch, 11. Mai 2016 12:25
> >>>> To: Langer, Christoph <christoph.lan...@sap.com>
> >>>> Cc: Alan Bateman <alan.bate...@oracle.com>; Dmitry Samersoff
> >>>> <dmitry.samers...@oracle.com>; net-dev@openjdk.java.net
> >>>> Subject: Re: RFR(S): 8156521: Minor Fixes and cleanups in
> >> NetworkInterface.c
> >>>>
> >>>>
> >>>> On 11 May 2016, at 10:21, Langer, Christoph
> <christoph.lan...@sap.com>
> >>>> wrote:
> >>>>
> >>>>> Hi,
> >>>>>
> >>>>> @Chris: As for your points:
> >>>>>
> >>>>>> I agree with the replacement of strcpy with strncpy, but I think we
> should
> >>>>>> explicitly null terminate in case the src is greater or equal to the
> >>>>>> dst
> >> buffer
> >>>>>> size. This is done elsewhere in this file too, e.g.
> >>>>>>
> >>>>>> strncpy(buf, input, sizeof(buf) - 1);
> >>>>>> buf[sizeof(buf) - 1] = '\0';
> >>>>>
> >>>>> There are 2 different patterns of strncpy usage in this code. One is
> where
> >> we
> >>>> would use strncpy giving the full buffer length and eventually setting
> >>>> the
> last
> >>>> byte to 0 to make sure the string is terminated. The other pattern is
> where a
> >>>> memset to 0 of the buffer was done before. So I thought it's fine here to
> give
> >>>> strncpy "buffersize - 1" as length since per its documentation it would
> copy
> >> all
> >>>> characters up to bufferlen and not terminate with 0 if strlen is >=
> bufferlen.
> >>>> That makes sense?
> >>>>
> >>>> It does. It was not clear to me that we were calling memset for ALL of
> these
> >>>> cases
> >>>> when I made the comment, but I carefully checked all your changes from
> >> strcpy
> >>>> to
> >>>> strncpy, and they do indeed look fine.
> >>>>
> >>>>>>>> Apart from quite a few whitespace changes to clean up the coding, I
> >> went
> >>>>>>>> through and replaced all occurences of strcpy with strncpy as this
> was a
> >>>>>>>> finding of a code scanner that we used. Also in function
> >>>>>>>> "enumIPv6Interfaces" for Linux the local variable plen was changed
> >> from
> >>>>>>>> int to short.
> >>>>>>
> >>>>>> Why was this done for plen specifically, and not scope, or others ?
> >>>>>
> >>>>> In struct _netaddr the mask field is defined as short and hence short is
> >>>> expected by the addif function. So plen should be a short in
> >> enumIPv6Interfaces
> >>>> for Linux, too.
> >>>>
> >>>> Ok, that's fine.
> >>>>
> >>>>> While looking again I see that the BSD function "static int prefix(void
> *val,
> >> int
> >>>> size)" should also rather be a "static short prefix(void *val, int
> >>>> size)". Shall
> I
> >>>> update this as well?
> >>>>
> >>>> Probably.
> >>>>
> >>>>> @Alan: As for the line length: If I get the feedback on those 2 points
> >>>>> I'll
> >>>> prepare a new webrev to shorten some lines.
> >>>>
> >>>> That would be great, thanks.
> >>>>
> >>>> -Chris.
> >
