c - Copying n chars with strncpy more efficiently in C

Question

I'm wondering if there's a cleaner and more efficient way of doing the following strncpy considering a max amount of chars. I feel like am overdoing it.

int main(void)
{

        char *string = "hello world foo!";
        int max = 5;

        char *str = malloc (max + 1);
        if (str == NULL)
                return 1;
        if (string) {
                int len = strlen (string);
                if (len > max) {
                        strncpy (str, string, max);
                        str[max] = '\0';
                } else {
                        strncpy (str, string, len);
                        str[len] = '\0';
                }
                printf("%s\n", str);
        }
        return 0;
}

Answer 1

I wouldn't use strncpy for this at all. At least if I understand what you're trying to do, I'd probably do something like this:

char *duplicate(char *input, size_t max_len) {
    // compute the size of the result -- the lesser of the specified maximum
    // and the length of the input string. 
    size_t len = min(max_len, strlen(input));

    // allocate space for the result (including NUL terminator).
    char *buffer = malloc(len+1);

    if (buffer) {
        // if the allocation succeeded, copy the specified number of 
        // characters to the destination.
        memcpy(buffer, input, len);
        // and NUL terminate the result.
        buffer[len] = '\0';
    }
    // if we copied the string, return it; otherwise, return the null pointer 
    // to indicate failure.
    return buffer;
}

Answer 2

Firstly, for strncpy, "No null-character is implicitly appended to the end of destination, so destination will only be null-terminated if the length of the C string in source is less than num."

We use memcpy() because strncpy() checks each byte for 0 on every copy. We already know the length of the string, memcpy() does it faster.

First calculate the length of the string, then decide on what to allocate and copy

int max = 5;               // No more than 5 characters

int len = strlen(string);  // Get length of string
int to_allocate = (len > max ? max : len); // If len > max, it'll return max. If len <= max, it'll return len. So the variable will be bounded within 0...max, whichever is smaller

char *str = malloc(to_allocate + 1); // Only allocate as much as we need to
if (!str) { // handle bad allocation here }

memcpy(str,string,to_allocate); // We don't need any if's, just do the copy. memcpy is faster, since we already have done strlen() we don't need strncpy's overhead

str[to_allocate] = 0; // Make sure there's a null terminator

Answer 3

Basically you're reinventing the strlcpy that was introduced in 1996 - see the strlcpy and strlcat - consistent, safe, string copy and concatenation paper by Todd C. Miller and Theo de Raadt. You might have not heard about it because it was refused to be added to glibc, called “horribly inefficient BSD crap” by the glibc maintainer and fought to this day even when adopted by all other operating systems - see the Secure Portability paper by Damien Miller (Part 4: Choosing the right API).

You can use strlcpy on Linux using the libbsd project (packaged on Debian, Ubuntu and other distros) or by simply copying the source code easily found on the web (e.g. on the two links in this answer).

But going back to your question on what would be most efficient in your case, where you're not using the source string length here is my idea based on the strlcpy source from OpenBSD at http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/lib/libc/string/strlcpy.c?rev=1.11 but without checking the length of the original string, which may potentially be very long but still with proper '\0' ending:

char *d = str;            // the destination in your example
const char *s = string;   // the source in your example
size_t n = max;           // the max length in your example

/* Copy as many bytes as will fit */
if (n != 0) {
    while (--n != 0) {
        if ((*d++ = *s++) == '\0')
            break;
    }
}

/* Not enough room in dst, add NUL */
if (n == 0) {
    if (max != 0)
        *d = '\0';      /* NUL-terminate dst */
}

Here is a version of strlcpy on http://cantrip.org/strlcpy.c that uses memcpy:

/*
 * ANSI C version of strlcpy
 * Based on the NetBSD strlcpy man page.
 *
 * Nathan Myers <ncm-nospam@cantrip.org>, 2003/06/03
 * Placed in the public domain.
 */

#include <stdlib.h>  /* for size_t */

size_t
strlcpy(char *dst, const char *src, size_t size)
{
    const size_t len = strlen(src);
    if (size != 0) {
        memcpy(dst, src, (len > size - 1) ? size - 1 : len);
        dst[size - 1] = 0;
    }
    return len;
}

Which one would be more efficient I think depends on the source string. For very long source strings the strlen may take long and if you don't need to know the original length then maybe the first example would be faster for you.

It all depends on your data so profiling on real data would the only way to find out.

Answer 4

You can reduce the volume of code by:

int main(void)
{
    char *string = "hello world foo!";
    int max = 5;

    char *str = malloc(max + 1);
    if (str == NULL)
        return 1;
    if (string) {
        int len = strlen(string);
        if (len > max)
            len = max;
        strncpy(str, string, len);
        str[len] = '\0';
        printf("%s\n", str);
    }
    return 0;
}

There isn't much you can do to speed the strncpy() up further. You could reduce the time by using:

char string[] = "hello world foo!";

and then avoid the strlen() by using sizeof(string) instead.

Note that if the maximum size is large and the string to be copied is small, then the fact that strncpy() writes a null over each unused position in the target string can really slow things down.

Answer 5

strncpy() will automatically stop once it hits a NUL; passing max without checking is enough.

Answer 6

I believe this is sufficient:

char *str = malloc(max+1);
if(! str)
return 1;

int len = strlen(string);  
memset(str, 0, max+1);
int copy = len > max ? max : len;
strncpy(str, string, copy);