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

Chris Hegarty chris.hegarty at oracle.com
Wed May 11 10:25:01 UTC 2016


On 11 May 2016, at 10:21, Langer, Christoph <christoph.langer at 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.


More information about the net-dev mailing list