RFR(S): 8156521: Minor Fixes and cleanups in NetworkInterface.c

Langer, Christoph christoph.langer at sap.com
Wed May 11 09:21:10 UTC 2016


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?

> >> 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. 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?

@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.
@Dmitry: Will also work in your findings in the update...

Thanks for all the feedback
Christoph



More information about the net-dev mailing list