How to: Safeguard memory - strncat()?

Question

function(char *a, char *b)
{

   char newStr[100];

   strncpy(newStr, a, sizeof(newStr)); //Line 1 - copy no more than 100 bytes  

   strncat(newStr, b, (sizeof(newStr) -  strlen(newStr)));   //Line 2 - ?

   newStr[99] = NULL; //Line 3 - null terminate string

}

Line 2: Correct to specify 100 bytes minus strlen of what a copied over from a to ensure I don't copy over the 100 bytes?

Thank You.

Answer 1

This is almost correct. First, the NUL-termination line:

newStr[99] = NULL;

is wrong.

strncat always NUL-teriminates, and the third parameter is the maximum number of bytes to write NOT including the NUL.

Hypothetically, if strncat didn't NUL-terminate, the problem with that line is that it would always write to the last element of the array, even though the actual string could be much shorter. If a and b were "Hello, " and "world!", the final array would be:

H|e|l|l|o|,| |w|o|r|l|d|!|g|i|b|b|e|r|i|s|h

where gibberish represents the previous contents of the array at those positions. Only at 99, after most of these uninitialized remnants, would there be a NUL.

EDIT: Also, Keith is correct about strncpy. His function is partly right, but that the second function can overflow the buffer, since it doesn't take into account the string that's already there. The two lines combined can write 199 characters (including NUL).

Also, I was wrong about the third parameter including the NUL. That leaves us with (modified from Keith's):

void function(char *a, char *b)
{
    char newStr[100];

    /* Make newStr an empty string so you can catenate onto it */
    newStr[0] = '\0';
    strncat(newStr, a, sizeof newStr - 1);
    strncat(newStr, b, sizeof newStr - strlen(newStr) - 1);

    /* Presumably you do something with newStr here */
}