I don't have an issue with the change but why is that more correct?

So, several things. But long story short, for all intend and purposes, what the code was doing is effectively a memcpy.

-) strncpy is used to copy a string with a max string limit - aka, you don't know in advance how many bytes you want to copy, but you know you don't want more than 'n' bytes to be copied. This isn't your case here: you know already how many bytes you have.
-) you've already done the strlen before - you know the input length, and strncpy is only going to re-test for the string '\0' at every char for quite no good reason.
-) you're at the max size of the string anyway, so you're in fact hitting the "bad behavior" of strncpy:

$ man strncpy
[...]
Warning: If there is no null byte among the first n bytes of src, the string placed in dest will not be null-terminated.

Some security-checker tools might start yelling at that, as doing a strncpy at its max, and not immediately adding a zero behind it is considered an obvious memory exploit.

-) for that bad behavior reason, strncmp is considered harmful, and shouldn't be used, unless you really know what you're doing. It's in fact flagged as depreciated under win32's libraries.

And I'm the security guy. hehe :). LGTM.

On Wed, Jan 28, 2015 at 7:05 PM, Nicolas Noble notifications@github.com
wrote:

So, several things. But long story short, for all intend and purposes,
what the code was doing is effectively a memcpy.

-) strncpy is used to copy a string with a max string limit - aka, you
don't know in advance how many bytes you want to copy, but you know you
don't want more than 'n' bytes to be copied. This isn't your case here: you
know already how many bytes you have.
-) you've already done the strlen before - you know the input length, and
strncpy is only going to re-test for the string '\0' at every char for
quite no good reason.
-) you're at the max size of the string anyway, so you're in fact hitting
the "bad behavior" of strncpy:

$ man strncpy
[...]
Warning: If there is no null byte among the first n bytes of src, the
string placed in dest will not be null-terminated.

Some security-checker tools might start yelling at that, as doing a
strncpy at its max, and not immediately adding a zero behind it is
considered an obvious memory exploit.

-) for that bad behavior reason, strncmp is considered harmful, and
shouldn't be used, unless you really know what you're doing. It's in fact
flagged as depreciated under win32's libraries.

—
Reply to this email directly or view it on GitHub
#273 (comment).