C - Use of strcpy in general

Question

I have this snippet of code:

new->name = zalloc(sizeof(char) * strlen(name) + 1);
if (!new->name)
    goto alloc_failed;
strcpy(new->name, name);

Is the general use if strcpy() frowned upon even if space for name was pre-allocated. Just wondering for secure purposes. Is is better to use strncpy() or srtlcpy().

Answer 1

Your coding example has some small issues:

new->name = zalloc(sizeof(char) * strlen(name) + 1);
if (!new->name)
    goto alloc_failed;
strcpy(new->name, name);

The size allocated is computed as sizeof(char) * strlen(name) + 1. If you insist on multiplying by sizeof(char) which by definition equals 1, you should at least write: sizeof(char) * (strlen(name) + 1). Barring that, your use of strcpy is fine in this example, but it would be more efficient to store the size in a local variable and use memcpy:

{
    size_t size = strlen(name) + 1;
    new->name = zalloc(size);
    if (!new->name)
        goto alloc_failed;
    memcpy(new->name, name, size);
}

Also note that POSIX systems have strdup() that does just the same thing using malloc. If zalloc() is a thin wrapper on malloc() that allocates and initializes the memory to zero from a single size argument, you can use strdup() as a simple replacement for this code. If it is a custom memory allocation scheme, you should also provide zstrdup() as duplicating character strings is common place in many C programs.

Another small remark on style: using goto is fine here if the local coding conventions condone it, but it would be preferable not to use obvious C++ keywords such as new and delete as plain identifiers.

Regarding strlcpy(), it may or may not be available on your platform but is a good option for other places. strncpy() OTOH is best avoided. It is a standard C function, but its semantics are so inconsistent with the rest of the C library that it is very often misused:

• The famous shortcoming is strncpy() does not tack a '\0' at the end of the destination if the source string is longer than the size parameter. snprintf(dest, size, "%s", src); would do that but is probably not very efficient.

• A less known side effect of strncpy occurs when the source string is shorter than the size parameter: the destination array is padded with '\0' all the way to the end. Using strncpy() to avoid buffer overflows is very inefficient if the destination array is substantially longer than the source.

Answer 2

You should use strcpy when you know what the max limit of the data possible is, and strncpy when it is user defined.

Your use of strcpy in this example is ok.