CODEWITHSUNDEEP
clinuxstring
Timothy Vann
Timothy Vann

Reputation: 2667

c snprintf used to concat string

I'm using the following function on two different computers. One one computer is running Ubuntu and the other OS X. The function works on OS X, but not Ubuntu.

#include <stdio.h>

#define MAXBUF 256

char *safe_strncat(char *dest, const char *src, size_t n) {
    snprintf(dest, n, "%s%s", dest, src);
    return dest;
}

int main(int argc, const char * argv[]){
    char st1[MAXBUF+1] = "abc";
    char st2[MAXBUF+1] = "def";
    char* st3;
    printf("%s + %s = ",st1, st2);
    
    st3 = safe_strncat(st1, st2, MAXBUF);
            
    printf("%s\n",st3);
    printf("original string = %s\n",st1);
}

Compile and run on Ubuntu

gcc concat_test.c -o concat_test

./concat_test

abc + def = def

original string = def

Compile and run in Xcode in OS X

abc + def = abcdef

original string = abcdef

  1. Why does this work on mac and not on Ubuntu?
  2. Should it work on Ubuntu?
  3. Should it work on Mac?
  4. I could swear it used to work in Ubuntu until recently, but I don't know what would have changed to make it stop working?
  5. Could compiler settings have anything to do with this working or not?

Upvotes: 2

Views: 1368

Answers (2)

chqrlie
chqrlie

Reputation: 144951

Your code invokes undefined behavior because you pass the destination buffer as one of the source strings of your snprintf() format. This is not supported:

7.21.6.5 The snprintf function

Synopsis

#include <stdio.h>

int snprintf(char * restrict s, size_t n,
             const char * restrict format, ...);

Description

The snprintf function is equivalent to fprintf, except that the output is written into an array (specified by argument s) rather than to a stream. If n is zero, nothing is written, and s may be a null pointer. Otherwise, output characters beyond the n-1st are discarded rather than being written to the array, and a null character is written at the end of the characters actually written into the array. If copying takes place between objects that overlap, the behavior is undefined.

(emphasis mine).

The implementation of snprintf differs between Ubuntu (glibc) and OS/X (Apple libc, based on BSD sources). The behavior differs and cannot be relied upon as it is undefined in all cases.

You can implement safe_strcat() this way:

#include <string.h>

char *safe_strcat(char *dest, size_t size, const char *src) {
    char *p = memchr(dest, '\0', size);
    if (p != NULL) {
        strncat(p, src, size - (p - dest) - 1);
    }
    return dest;
}

Notes:

  • do not call this function safe_strncat(), it is really a safe version of strcat().
  • pass the size of the destination array after the destination pointer itself, not after the source pointer.
  • returning the destination pointer does not allow the caller to detect truncation. You could instead return the length of the result if the destination array had been large enough, like snprintf(), you it would still not tell the caller if the destination was not null terminated before the call (for safe_strcat and safe_strncat).

You can use the same model for safe versions of strcpy, strncat and strncpy (but not implementing strncpy()'s counter-intuitive semantics):

char *safe_strcpy(char *dest, size_t size, const char *src) {
    if (size > 0) {
        *dest = '\0';
        strncat(dest, src, size - 1);
    }
    return dest;
}

char *safe_strncat(char *dest, size_t size, const char *src, size_t n) {
    char *p = memchr(dest, '\0', size);
    if (p != NULL) {
        if (n > size - (p - dest) - 1)
            n = size - (p - dest) - 1;
        strncat(p, src, n);
    }
    return dest;
}

char *safe_strncpy(char *dest, size_t size, const char *src, size_t n) {
    if (size > 0) {
        if (n > size - 1)
            n = size - 1;
        *dest = '\0';
        strncat(dest, src, n);
    }
    return dest;
}

Upvotes: 6

tdao
tdao

Reputation: 17678

snprintf(dest, n, "%s%s", dest, src);

This line caused undefined behaviour because you are overwriting the buffer dest. Because it's not defined, it's pointless to work out why it works on one machine and not the other.

More details can be found here: Is sprintf(buffer, "%s […]", buffer, […]) safe?

Upvotes: 2

Related Questions