Dynamically increase the dimension of an array of characters

Question

I have this function in C which I use in my project for reading characters in input:

char* readStringInp()
{
    char* string=NULL,c=NULL;
    int i=0;

    string=(char *)malloc(sizeof(char)*LGTH_STRING_INPUT);

    for(i=0;i<LGTH_STRING_INPUT && ( (c=getchar()) != '\n' && c != EOF );i++)
        string[i]=c;

    string[i]='\0';

    return string;
}

LGTH_STRING_INPUT is a numerical constant which represent the maximum length of the string that can be read. Which is the best way to read a string with no fixed length?

please format and indent your code nicely, and google `realloc`. — Jason Hu, Mar 23 '15 at 13:53
In ANSI C it is not correct to cast the return of malloc() (or calloc() or realloc() ). — ryyker, Mar 23 '15 at 13:56
Also `c` should be defined `int`, at least to be able to successfully detect `EOF`. `getchar()` returns `int`, btw. — alk, Mar 23 '15 at 13:58
Your code causes a buffer overflow. When you malloc `N` bytes the valid indices are `0` through `N-1`. But you go on to set `string[N]` at the end. — M.M, Mar 23 '15 at 14:10

user3121023 · Answer 1 · 2015-03-24T15:54:57.223

2

This allocates memory for 20 characters. When 18 characters have been used, another 20 characters are allocated by realloc. As the string grows, additional increments of 20 characters will be allocated. If realloc fails, the current string is returned. The while loop terminates when a newline or EOF is found.
The allocated memory should be freed by the calling function.
EDIT added main() to show the freeing of the allocated memory

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

char* readStringInp();

int main()
{
    char *s=readStringInp();
    if ( s != NULL)
    {
        printf ( "%s\n", s);
        free ( s);
    }
    return 0;
}

char* readStringInp()
{
    char* string=NULL, *temp = NULL;
    int c=0, available=20, used=0, increment=20;

    string=malloc(available);
    if ( string == NULL)
    {
        printf ( "malloc failed\n");
        return NULL;
    }

    while( (c=getchar()) != '\n' && c != EOF )
    {
        if ( used == available - 2)
        {
             available += increment;
             temp = realloc ( string, available);
             if ( temp == NULL)
             {
                 printf ( "realloc failed\n");
                 return string;
             }
             string = temp;
        }
        string[used]=c;
        used++;
        string[used]='\0';
    }
    return string;
}

edited Mar 24 '15 at 15:54

answered Mar 23 '15 at 17:16

user3121023

8,181
5
18
16

Where do you handle freeing memory once it is no longer needed? The calling function cannot free string as it is local to this function. – ryyker Mar 23 '15 at 17:39
This solution work like a charm and It's what I really needed, thank you so much :) – Richard Temp Mar 24 '15 at 11:12
@RichardTemp - If you like an answer, up-click it. (little arrows to the upper left of each answer) If it answers your question, click the hollow checkmark. – ryyker Mar 24 '15 at 14:10
@ryyker I know that, but I need repution 15 to upvote :/ – Richard Temp Mar 24 '15 at 15:08
@RichardTemp - Ooops. Forgot about that little detail. Its odd that the site restricts someone from upclicking an answer to one of their own questions, no matter their rep, don't you think? – ryyker Mar 24 '15 at 18:30
@ryyker Yes you're right. When my reputation will be > 15 I'll upvote your anwser, promised! – Richard Temp Mar 27 '15 at 18:18
@ user3121023 I correctly upvoted your answer :D I did a mistake in the previous answer ,soryy :P – Richard Temp Mar 27 '15 at 21:07
You should accept this answer if it solved your problem. There is a difference between accept and upvote. You can always accept regardless of your reputation. See [this](https://meta.stackexchange.com/a/5235) post post how to do it. – Programmer Aug 27 '17 at 18:38

score 1 · Answer 2 · edited May 23 '17 at 12:30

1

Which is the best way to read a string with no fixed length?

As already done in your code allocate memory dynamically using malloc() and keep reading the characters until the sentinel condition is reached and keep increasing the memory allocated using realloc() on the same pointer unless you have space to hold all the characters.

Don't cast malloc and family

edited May 23 '17 at 12:30

Community

1
1

answered Mar 23 '15 at 13:53

Gopi

19,784
4
24
36

ryyker · Answer 3 · 2015-03-23T21:13:27.053

A combination of calloc() (which you are already using) and realloc() would be used to dynamically size your buffers to accommodate various lengths of user input.
However, there are several issues in your existing example code that should be addressed before going on to use that combination: See suggestions and comments below:

As written, your existing code will leak memory as you never call free(). Also, you will get a buffer overrun in string. (see explanation below)

Here ere are a couple of corrections that will allow your example code to work: (see comments)

#define LGTH_STRING_INPUT 20

//have written function to accept argument, easier to free memory
char* readStringInp(char *string){

    //char* string=NULL,c=NULL;//done in main
    int c; //getchar() requires int to detect EOF (-1)
    int i=0;

    //string=malloc(LGTH_STRING_INPUT);  //do not cast return value of calloc

    //   max length allow should be LGTH_STRING_INPUT - 1 
    //   to allow room for NULL after loop 
    for(i=0;i<LGTH_STRING_INPUT-1 && ( (c=getchar()) != '\n' && c != EOF );i++)
    {
        string[i]=c;
    }

    string[i]='\0';

    return string;
}

int main(void)
{
    char *buf = {0};
    char bufIn[LGTH_STRING_INPUT];

    //allocated and free memory in calling function:
    buf = malloc(LGTH_STRING_INPUT);
    if(buf)//check return of malloc
    {
        sprintf(bufIn, "%s", readStringInp(buf)); //use return value in buffer
        free (buf);//free memory that has been created
    }
    return 0;
}

Thanks for this solution, but actually I prefer the first one, also because I don't want use sprintf .Thank you :D — Richard Temp, Mar 24 '15 at 11:12
@RichardTemp - thank you for commenting. The use of `sprintf()` was completely arbitrary, for demonstration. You can use any of the string functions to get the result of your function an place it into a buffer. `strcpy()`, `strcat()` are two examples that may work for you. IN any case, the point of this answer was three fold: 1) to point out correct usage of `malloc()`, 2) that memory cleanup should be done, and 3) to help you see a way to write the function in such a way that allows easy memory cleanup. — ryyker, Mar 24 '15 at 14:05
Ok got It . Just for say, I used free somewhere else in my program, that's why I didn't wrote it in my buggy piece of code.However I didn't know that cast used in that manner would create all of those troubles. — Richard Temp, Mar 24 '15 at 15:11
@RichardTemp - The cast, in this particular case, did not likely cause _any_ troubles for you here. It is just not necessary for ANSI C, and _can_ cause problems. That the only reason I mentioned it. — ryyker, Mar 24 '15 at 18:27

Dynamically increase the dimension of an array of characters

3 Answers3