Allocating memory for a string in C using malloc

Question

I am trying to allocate memory for an array of strings using malloc. The size of each string is not known before the input from the user, so this is how I tried to allocate memory for each element in the array.

I have some errors with the code, but can't figure them out or can't understand them. I am getting an error regarding the allocation. Can anyone tell me what's wrong about this?

bool read_strings(char * strings[], int n) 
{
    int i = 0;
    while (i<n)
    {
        char string[MAX_LENGTH];
        if (scanf("%s", string)!=1)
            return false;
        char* memory= (char*)malloc(sizeof(char)*strlen(string));
        if (memory == NULL)
            return false;
        memory = string;
        strings[i] = memory;
        i++;
    }
    return true;
}

Thanks a lot!

Answer 1

At least you have to replace

char* memory= (char*)malloc(sizeof(char)*strlen(string));
if (memory == NULL)
    return false;
memory = string;
strings[i] = memory;

by

strings[i] = strdup(string)

Note that using scanf("%s", string) the separator between the read string is the space

Answer 2

You have many mistakes

1. (char*)malloc(sizeof(char)*(strlen(string) **+ 1**)) . You have to reserve memory to '\0'

2. Very wrong

   memory = string;

   To copy strings you have to usr strcpy (the correct function nowadays is strncpy is more safe)

Answer 3

To have a truly unlimited buffer in C, (or limited by the amount of memory and a size_t,) you could build up the memory allocation incrementally.

#include <stdlib.h>  /* realloc free */
#include <stdio.h>   /* stdin fgets printf */
#include <string.h>  /* strcpy */
#include <assert.h>  /* assert */
#include <stdint.h>  /* C99 SIZE_MAX */
#include <stdbool.h> /* C99 bool */

/* Returns an entire line or a null pointer, in which case eof or errno may be
 set. If not-null, it must be freed. */
static char *line(void) {
    char temp[1024] = "", *str = 0, *str_new;
    size_t temp_len, str_len = 0;
    while(fgets(temp, sizeof temp, stdin)) {
        /* Count the chars in temp. */
        temp_len = strlen(temp);
        assert(temp_len > 0 && temp_len < sizeof temp);
        /* Allocate bigger buffer. */
        if(!(str_new = realloc(str, str_len + temp_len + 1)))
            { free(str); return 0; }
        str = str_new;
        /* Copy the chars into str. */
        strcpy(str + str_len, temp);
        assert(str_len < SIZE_MAX - temp_len); /* SIZE_MAX >= 65535 */
        str_len += temp_len;
        /* If on end of line. */
        if(temp_len < sizeof temp - 1 || str[str_len - 1] == '\n') break;
    }
    return str;
}

static bool read_strings(char * strings[], int n) {
    char *a;
    int i = 0;
    while(i < n) {
        if(!(a = line())) return false;
        strings[i++] = a;
    }
    return true;
}

int main(void) {
    char *strings[4] = { 0 }; /* C99 */
    size_t i;
    bool success = false;
    do {
        if(!read_strings(strings, sizeof strings / sizeof *strings)) break;
        for(i = 0; i < sizeof strings / sizeof *strings; i++)
            printf("%lu: <%s>\n", (unsigned long)i, strings[i]);
        success = true;
    } while(0); {
        for(i = 0; i < sizeof strings / sizeof *strings; i++)
            free(strings[i]);
    }
    return success ? EXIT_SUCCESS : (perror("stdin"), EXIT_FAILURE);
}

I think that's right-ish. However, this should bring pause; what if they never hit enter? If one has a MAX_LENGTH, then consider allocating statically, depending on your situation.

Edit: It also has a worst-case running time that may not be desirable; if entering really arbitrarily large lines, use a geometric progression to allocate space.

Answer 4

The problem is right here:

char* memory = (char*)malloc(sizeof(char)*strlen(string));
memory = string; <<<
strings[i] = memory;

You will lose memory if you assign strings to pointers like this.

Either:

a) Copy the string into the newly allocated memory using strcpy() or strncpy(), also make sure you have enough space for the NULL character \0

strings[i] = (char*)malloc(sizeof(char) * (strlen(string) + 1));
strcpy(strings[i], string);

b) Use strdup(), which is like a mix between strcpy() and malloc(), it creates just enough space for your string and copies it into a new memory location

 strings[i] = strdup(string);

Answer 5

I think this is what you wanted to do:

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

int read_strings(char * strings[], int n)
{
    int i = 0;
    char buffer[256] ={0}; /*a temp buffer of fixed max size for input */
    if(NULL == strings)
    {
        return 0 ;
    }

    for (i= 0; i<n; ++i)
    {
      if (fgets(buffer, 256,stdin)== NULL) /*safer then scanf - read input into the buffer*/
        return 0;
        strings[i]= malloc(sizeof(char)*(strlen(buffer)+1)); /* the char poiner in he i place will now point to the newly allocated memory*/
        strcpy(strings[i], buffer); /*copy the new string into the allocated memory now string[i] is pointing to a string*/
    }

return 1;
}
static void printStringsArray(const char* strArr[], size_t size)
{
    int i = 0;
    if(NULL == strArr)
    {
        return;
    }
    for(i = 0; i< size; ++i)
    {
        printf("%s", strArr[i]);
    }

}

 int main(void)
 {
    char * arr[3]; /*array of (char*) each will point to a string after sending it to the function */
    read_strings(arr,3);

    printStringsArray(arr,3);
    return 0;
 }