0

like the title already states I would like to copy various strings into a two-dimensional array. Each string has a different size therefore I need to use memory reallocation. The code below should do this job, but somehow I cannot get it working.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main(void) {

char **str = NULL;
int str_num = 1;

// get string
char *str_new = "hey mate\0";

printf("%s\n",str_new);

// reallocation for str_new
str = realloc(str,str_num*sizeof(char));
str[str_num-1] = realloc(str,strlen(str_new));

// copy string to new space
strcpy(*str, str_new);

// displaying string
printf("%s\n",str[str_num-1]);


return EXIT_SUCCESS;

}
Philipp Braun
  • 1,583
  • 2
  • 25
  • 41
  • Two things: The compiler automatically adds the string terminator `'\0'` to all string literals, and all string literals are read-only so a pointer to them should be `const`. – Some programmer dude Dec 29 '14 at 15:32

4 Answers4

4

One problem is in the reallocation:

str = realloc(str,str_num*sizeof(char));

You only allocate space for a single byte and not for a pointer to char. This leds to undefined behavior.

Change to e.g.

str = realloc(str,str_num*sizeof(char*));

Another problem, and also a cause for undefined behavior is the actual string allocation:

str[str_num-1] = realloc(str,strlen(str_new));

Here you are reallocating str and not str[0]. And you don't alocate space for the string terminator.

For this don't use realloc at all, since you're only allocating this once you only need either malloc, or just use the strdup function to do both allocation and copying in one call:

str[str_num - 1] = strdup(str_new);

By the way, when using realloc never assign to the same pointer you're passing in as first argument, because if the realloc function fails it will return NULL and you will loose the original pointer. Instead assign to a temporary pointer, and if it's non-null then assign to the actual pointer.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
1

Beside correcting (as pointed in Joachim Pileborg's answer)

str = realloc(str,str_num*sizeof(char));  

to

str = realloc(str,str_num*sizeof(*str));

you should note that, the pointer passed to realloc must be the pointer returned by any malloc family function or it points to NULL. You can do it as

str[str_num-1] = NULL;
str[str_num-1] = realloc(str[str_num-1], strlen(str_new)+1);
Community
  • 1
  • 1
haccks
  • 104,019
  • 25
  • 176
  • 264
  • @WhozCraig, you probably missed out the `*` in your suggested correction. Both statements are identical. – B.Shankar Dec 29 '14 at 15:57
  • 1
    @B.Shankar `strlen(str_new)` and `strlen(str_new)+1` are anything-but identical. I think you mean hacks original post, which yes, is missing the `*`. Added. – WhozCraig Dec 29 '14 at 16:05
1
char **str = malloc(sizeof(char *) * n); /* n= number of pointers */

Allocate memory for the pointer's first then allocate memory to individual pointer like

   for(i=0;i<n;i++)
   {
       str[i] = malloc(sizeof(char) * 20);
       /* copy your string to the allocated memory location here*/
   }

Else you can have

char **str = NULL;

str = realloc(str,str_num * sizeof(char *));
Gopi
  • 19,784
  • 4
  • 24
  • 36
1

I think you don't understand what realloc does, the code you posted is not right, that code should be written this way

char **str = NULL;
int str_num = 1;

// get string
char *str_new = "hey mate\0";

printf("%s\n",str_new);

/*
    // reallocation for str_new
    str = realloc(str, str_num * sizeof(char));
 */ 
/* allocate space for `str_num` pointers of char */
str = malloc(str_num * sizeof(char *));
if (str == NULL) /* check it worked */
    return -1;

/* allocate space for the number of characters in str_new and
 * the termination null byte 
 */
str[str_num - 1] = malloc(1 + strlen(str_new));
if (str[str_num - 1] == NULL)
{
    free(str);
    return -1;
}
/*
 * // copy string to new space
 * strcpy(*str, str_new);
 */
/* use the same index since if str_num > 1 the above is wrong */
strcpy(str[str_num - 1], str_new);

// displaying string
printf("%s\n",str[str_num - 1]);

realloc should be used like this

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main(void) 
{
    char **str = NULL;
    int str_num;
    // get string
    char *str_new[2] = {"hey mate 1", "hey mate 2"};

    for (str_num = 0 ; str_num < 2 ; ++str_num)
    {
        char **pointer;
        printf("%s\n", str_new[str_num]);
        /*
            // reallocation for str_new
            str = realloc(str, str_num * sizeof(char));
         */ 

        /* re-allocate space for `str_num` pointers of char */
        pointer = realloc(str, (1 + str_num) * sizeof(char *));
        /* 
         * If you use
         * 
         *      str = realloc(str, str_num * sizeof(char *));
         * 
         * you wont be able to free(str) on failure
         */
        if (pointer == NULL)
        {
            int j;
            /* on failure cleanup and return */
            for (j = str_num - 1 ; j >= 0 ; --j)
                free(str[j]);
            free(str);

            return -1;
        }
        str = pointer;      

        /* allocate space for the number of characters in str_new and
         * the termination null byte 
         */
        str[str_num] = malloc(1 + strlen(str_new[str_num]));
        if (str[str_num] == NULL)
        {
            free(str);
            return -1;
        }
        /*
         * // copy string to new space
         * strcpy(*str, str_new);
         */
        /* use the same index since if str_num > 1 the above is wrong */
        strcpy(str[str_num], str_new[str_num]);

        // displaying string
        printf("%s\n",str[str_num]);
    }

    return EXIT_SUCCESS;
}

you should remember to free all the allocated memory.

Also you don't need to embed the '\0' in a string literal.

Iharob Al Asimi
  • 52,653
  • 6
  • 59
  • 97