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.
