Search code examples
caliasstdio

Is it safe to have asprintf use a non-NULL target pointer that is also a input?

tl;dr Can asprintf be used naively for concatenation without invoking a temporary pointer?

The function asprintf introduced by GNU and adopted in several other clib implementations is a tempting solution for arbitrary concatenation in c using a scheme like

int i=0;
char *str = strdup(argv[i]);
while (argv[++i]) {
   asprintf(&str,"%s %s",argv[i],str);   // <=== This line
}
asprintf(&str,"%s\n",str);

When wrapped in a main and the necessary includes, this runs fine for me.

But...

Is it leaking memory all over the place? Valgrind says it is on my box. Is this a bug?

The man-page that I have in front of me says

The asprintf() and vasprintf() functions set *ret to be a pointer to a buffer sufficiently large to hold the formatted string. This pointer should be passed to free(3) to release the allocated storage when it is no longer needed. If sufficient space cannot be allocated, asprintf() and vasprintf() will return -1 and set ret to be a NULL pointer.

In the absense of the the phrase "set *ret to be a pointer to a new buffer [...]", I am tempted to assume that the function is using realloc as getline does.

What might be a problem?

Using the signature int asprintf(char **ret, const char *format, ...); for concreteness.

  1. asprintf runs realloc too soon.

    Imagine that we implement the function in such a away that we could run realloc(*ret) before it dereferences one of the varargs that aliased the original buffer. That buffer has been freed and this is technically undefined behavior. This would represent a bug.

  2. asprintf writes into the buffer before it reading it. In the above code we can imaging the function copying the contents of argv[1] into *ret before it every va_arg on the str argument. The manpage I quote does not seem to rule this case out.

  3. asprintf does not free *ret either directly or through the use of realloc. This would avoid problem number (1) but would leak memory if used as above. Again that man page seems not to rule it out.

Work around

All of the above could be avoided by replacing the single call to asprintf with

{
  char *newStr = NULL;
  asprintf(newStr,"%s %s",argv[i],str);
  free(str);
  str = newStr;
}

but that is pretty clunky.

Does the consensus implementation guarantee that the first code sample is safe and correct?

Solution

  • Is it leaking memory all over the place?

    Yes, it does.

    char *str = strdup(argv[i]);

    Here, str contains a pointer to malloc()ated memory which should be free()d.

    asprintf(&str, "%s %s", argv[i], str);

    Here, asprintf() modifies str so that it points to some other memory allocated by the function itself. Now you just lost the pointer to the strdup()ped string, hence the leak.