Home > database >  Is this snprintf method a safe way to copy strings?
Is this snprintf method a safe way to copy strings?

Time:12-15

I need your advice. I use this way to copy strings knowing a max size to not go over but some of these strings don't end with a null terminator. It's just a snippet.

void my_strcpy(char* dest, const char* src, const size_t max_size)
{
    snprintf(dest, max_size, "%.*s", max_size - 1, src);
}

Is there a safer method? strlcpy? (I've not handled errors here.)

CodePudding user response:

OP's use looks OK - perhaps a bit pedantic with a size limit and precision.

"%.*s" tolerates a pointer to a character array with no null character. Copying will limit to the precision or the null character, whichever is first.

Resultant dest[] should never be not null-character terminated with valid arguments.

I'd add some tests:

// return error code
int my_strcpy(char* dest, const char* src, size_t max_size) {
  assert(dest && src && max_size && max_size < INT_MAX); // Validate arguments.
  //                                         v-- use int here --v
  int len = snprintf(dest, max_size, "%.*s", (int) (max_size - 1), src);
  return (len < 0 || (unsigned) len >= max_size);
}

I suspect sprintf(dest, "%.*s", max_size - 1, src); is sufficient too, expect it lacks detection of an overly long src.


Of course one could use something like

void my_strcpy(char* restrict dest, const char* restrict src, size_t size) {
  if (dest && src && size) {
    while (--size > 0 && *src) {
      *dest   = *src  ;
    }
    *dest = 0;
  }
}

restrict here implies src/dest do not overlap.

CodePudding user response:

Your approach is not completely safe for multiple reasons:

  • The precision argument for snprintf %.*s specifier must be cast as an int. If size_t has a different size on your target system, the behavior is undefined. You should use a cast (int)(max_size - 1).
  • Your approach will fail if max_size > INT_MAX even with a cast.
  • If max_size == 0, snprintf() might still read bytes from src, which might cause undefined behavior especially if the source string is not null terminated.
  • If the source string is not correctly encoded according to the currently selected locale, snprintf might stop the copy and return -1, leaving the destination without a null terminator.

It is not clear what you are trying to achieve:

I use this way to copy strings knowing a max size to not go over but some of these strings don't end with a null terminator

Is max_size a maximum number of bytes to copy as for strncat or the length of the destination array including space for the null terminator as for snprintf?

What strings do not have a null terminator? the source strings or the resulting contents of the destination array?

As coded, the argument max_size is the length of the destination array, including the null terminator.

To avoid the above issues, here are some standalone alternatives:

A truncating version that is very easy to use as

char array[SIZE];
my_strcpy_trunc(array, sizeof array, source);
// copy source string to an array of length size
// truncate contents to fit in the destination array
// return 0 if successful and no truncation occurred
// return 1 if truncation occurred
// return 2 if src is NULL, destination set to an empty string
// return -1 if arguments are invalid, no copy occurred
int my_strcpy_trunc(char *dest, size_t size, const char *src) {
    if (dest && size) {
        if (src) {
            for (;;) {
                if ((*dest   = *src  ) == '\0')
                    return 0;  // success
                if (--size == 0) {
                    dest[-1] = '\0';
                    return 1;  // truncation occurred
                }
            }
        }
        *dest = '\0';
        return 2;  // src is null pointer
    } else {
        return -1; // invalid dest
    }
}

A limiting version similar to strncat where you assume the destination has at least n 1 bytes available, a less safe approach IMHO:

// copy source string up to a maximum of n bytes and set the null terminator to an array of length size
// return 0 if successful
// return 1 if successful and src length larger than n
// return 2 if src is NULL, destination set to an empty string
// return -1 if arguments are invalid, no copy occurred
int my_strcpy_limit(char *dest, const char *src, size_t n) {
    if (dest) {
        if (src) {
            while (n --> 0) {
                if ((*dest   = *src  ) == '\0')
                    return 0;  // success
            }
            *dest = '\0';
            return *src ? 1 : 0;
        }
        *dest = '\0';
        return 2;  // src is null pointer
    } else {
        return -1; // invalid dest
    }
}
  • Related