Realloc using stdarg

Question

I'm trying to concatenate strings using stdarg (~~library~~) header, but I'm doing something wrong. There is a easier way to concatenate strings using realloc?

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdarg.h>
void concatenaCodigo(char *string, char *format, ...){
    va_list args;
    va_start(args, format);
    int n = vsnprintf(NULL, 0, format, args);
    string = (char*) realloc (string, n);
    if (string != NULL) {
        printf("Realloc OK!\n");
        vsprintf(string, format, args);
        va_end(args);
    }
    else {
        free (string);
        puts ("Error (re)allocating memory");
        exit (1);
    }

}
int main(){
    char *codigoC = NULL;
    concatenaCodigo(codigoC, "Test%s%s","asd","asd");
    printf("%s\n", codigoC);
}

I made the changes. The code should look like the below? The concatenation is not working yet.

char* concatenaCodigo(char *format, ...){
    va_list args;
    va_start(args, format);
    int n = vsnprintf(NULL, 0, format, args);
    char * newString;
    newString = (char*) malloc(n);
    vsprintf(newString, format, args);
    va_end(args);
    return newString;
}
int main(){
    char *codigoC = NULL;
    codigoC = concatenaCodigo("Test%s%s", "asd", "asd");
    printf("%s\n", codigoC);
}

You are reallocating memory and then throwing it away. You need to return the new pointer from the function somehow. — n. m. could be an AI, Apr 29 '13 at 03:23
stdarg is not a library, it's part of the clib, and most of it if not all is just macros — stdcall, Apr 29 '13 at 04:55

score 3 · Answer 1 · answered Apr 29 '13 at 04:46

The problem in the revised code is that you're not resetting your va_list properly. You have:

char* concatenaCodigo(char *format, ...){
    va_list args;
    va_start(args, format);
    int n = vsnprintf(NULL, 0, format, args);
    char * newString;
    newString = (char*) malloc(n);
    vsprintf(newString, format, args);
    va_end(args);
    return newString;
}
int main(){
    char *codigoC = NULL;
    codigoC = concatenaCodigo("Test%s%s", "asd", "asd");
    printf("%s\n", codigoC);
}

You need:

char *concatenaCodigo(char *format, ...)
{
    va_list args;
    va_start(args, format);
    int n = vsnprintf(NULL, 0, format, args) + 1;  /* Note the +1! */
    va_end(args);                                  /* vsnprintf() 'uses up' args */
    char *newString = (char *) malloc(n);
    if (newString != 0)
    {
        va_start(args, format);                        /* Restart args */
        vsprintf(newString, format, args);
        va_end(args);
    }
    return newString;
}

int main(void)
{
    char *codigoC = concatenaCodigo("Test%s%s", "asd", "asd");
    if (codigoC != 0)
        printf("%s\n", codigoC);
    free(codigoC);
    return 0;
}

Note that <stdarg.h> is a header, not a library.

Thanks @Jonathan, it is working perfectly. I do not have much experience with C. It would take days to solve the problem. — Matheus Pereira, Apr 29 '13 at 17:43

score 0 · Answer 2 · answered Apr 29 '13 at 03:26

0

The pointer isn't passed by reference, so when you do:

void concatenaCodigo(char *string, char *format, ...){
    // ...
    string = (char*) realloc (string, n);
}

You're overwriting the pointer that was passed in, and never getting the new value out of the function.

concatenaCodigo should return the newly allocated string:

char* concatenaCodigo(char *format, ...) {
    // ...
    return string;
}

int main() {
    char *codigoC = concatenaCodigo("Test%s%s","asd","asd");
    printf("%s\n", codigoC);
}

Also, you should not call free(string) if realloc failed. You shouldn't pass a NULL pointer to free().

answered Apr 29 '13 at 03:26

Jonathon Reinhart

132,704
33
254
328

Please don't post code in comments; it is totally unreadable. Instead, [edit the original post](http://stackoverflow.com/posts/16270728/edit) and add it. – Jonathon Reinhart Apr 29 '13 at 04:00
2

This is not the correct way to call realloc(). If you use realloc(), you should use a temporary ptr to store the result. If you do not, and realloc() fails, you destroy the original. – Randy Howard Apr 29 '13 at 04:14
1

"Also, you should not call free(string) if realloc failed. You shouldn't pass a NULL pointer to free()." -- Neither of those claims is correct. Please read the documentation for both functions. As Randy says, the result of realloc should be stored in a temp. If the realloc fails, either string should be used with its current size or it should be freed. freeing NULL is a no-op. – Jim Balter Apr 29 '13 at 05:24
From the C standard: "If memory for the new object cannot be allocated, the old object is not deallocated and its value is unchanged." – Jim Balter Apr 29 '13 at 05:27

score 0 · Accepted Answer · answered May 06 '13 at 20:13

To avoid losing the reference I changed the function to receive a pointer to char array.

void concatenaCodigo(char **message, char *format, ...)
{
    va_list args;
    va_start(args, format);
    int n = vsnprintf(NULL, 0, format, args) + 1;  /* Note the +1! */
    va_end(args);                                  /* vsnprintf() 'uses up' args */
    char *newString = (char *) malloc(n);
    if (newString != 0)
    {
        va_start(args, format);                        /* Restart args */
        vsprintf(newString, format, args);
        va_end(args);
    }
    *message = newString;
}

What, in your estimation, is the advantage of passing the double pointer into the function rather than simply returning a pointer from the function? It couples the function more closely with its caller. — Jonathan Leffler, May 31 '13 at 09:56

Realloc using stdarg

3 Answers3