Dynamic string allocation in c

Question

The function I am having a problem with is the one I created below, DynamicString. The function is meant to create a string of variable size.

void DynamicString(char **txt)
{
    int i;
    int n = 0; //number of characters
    char c;
    *txt = (char *)malloc(1);//one byte of memory for the terminator
    (*txt)[0] = '0'; // termination charcter is the first point
    char *tmp = NULL; //temporary pointer

    scanf("%c", &c);
    while(c != '\n')//while true input
    {

        tmp = (char*)malloc(n+2);//allocates memory for incoming charcter and termination point
        for (i=0; i < n; ++i)//doesn't copy over an empty list
        {tmp[i] = (*txt)[i];}

        tmp[n] = c; //inputs character in releveant position
        tmp[n+1] = '0'; //places term. point
        ++n;// incriments the number of charcters(not including 0
        scanf("%c", &c);
    }

    free(*txt);//frees old mem
    *txt = tmp;// txt points to tmp
    printf("\nThe entered string is : %s",*txt);


    return;
}

void Frequency(char *s, int *array)
{
    int i;
    for(i=0; s[i] != '\0'; i++)//counts the frequncies of each character
    {
        array[s[i]]++;
    }
    /* Print characters and their frequency */

    return;
}

int main()
{
    //int Freq[128] = {0};// frequency of characters
    char * string;
    DynamicString(&string);

    free(string);

    return 0;
}

If anyone can fix the code I have already created that would be very helpful. I know there might be a better algorithm but I just want to know what I'am doing wrong. Thank you!

Code looks strange, one of the problem is incorrect string termination symbol. It should be '\0', but not '0'. — Nikita, Nov 11 '17 at 23:37
I think all of your `'0'` characters should really be `'\0'`, which is quite different. (I see two instances of `'0'` and one instance of `'\0'`) — Tom Karzes, Nov 11 '17 at 23:48
The problem was that i was not copying over the tmp string in the while loop. — Christopher Moskovitz, Nov 12 '17 at 13:23

score 1 · Answer 1 · answered Nov 12 '17 at 21:16

First of all, as others have pointed out, correct string end is '\0' which is a terminating NUL character.

Secondly, if the first character read happens to be the newline, the loop will never be executed. After you free(*txt); *txt points to tmp which is a NULL, so you cannot use printf("\nThe entered string is : %s",*txt); because you are dereferencing a NULL pointer which is undefined behaviour in c.

Thirdly, you have a memory leak there because in the loop you are allocating memory to tmp using malloc(), which every time will give you a new memory address. This means in every cycle in the loop, tmp will have a new address and you lose the old address and you cannot free it anymore.

If you want to increase or decrease the size of the memory a pointer points to, use realloc. A very simple example how to use realloc would be like

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


int main(void)
{
    int initalSize = 10;
    int increasedSize = 20;

    /*
     *  Because temp type is char, I left sizeof operator out. you can always write like
     *  char *sentence = malloc(initialSize * sizeof(*sentence)); or
     *  char *sentence = malloc(initialSize * sizeof(char));
     */
    char *sentence = malloc(initalSize);
    char *temp = NULL;

    /* 
     * Malloc can return a NULL pointer. You need to check sentence value before using it
     * elswhere in the code.
     */
    if (!sentence) {
      printf("Memory allocation failed for sentence\n");
      exit(EXIT_FAILURE);
    }

    // Some code here...

    /*
     * Realloc can also return a NULL pointer. Need to use a temporary
     * pointer. In case realloc really returns NULL, and you don't use a temporary pointer and
     * use it like "sentence = realloc(sentence, increasedSize);", you will have a memory leak
     * because now sentence = NULL and you don't have a pointer that points to the old memory.
     *
     */
    temp = realloc(sentence, increasedSize);

    // Also need to check temporary pointer value.
    if (!temp) {
      printf("Allocating more memory to sentence failed\n");

      // One of the possible solutions. You can always use the value that you already have in sentence;
      free(sentence);
      exit(EXIT_FAILURE);
    }

    /*
     * If everything was ok, you make sentence point to the same address in memory as temp was pointing.
     * What it means is that you give ownership of the memory temp points to the sentence.
     */
    sentence = temp;

    // Some code here...


    // No need for sentence anymore
    free(sentence);

    /*
     * It is always recomended to make unused pointers that you don't use anymore or if you have
     * freed them to point to NULL;
     * It makes sure that those pointers no longer point to the previous memory addresses. Remember that
     * free only frees the memory where sentence and temp pointed to.
     *
     * If you don't make them point to NULL, then sentence and temp would be called a dangling pointer.
     *
     */
    sentence = NULL;
    temp = NULL;

    // Some code here...

    return 0;
}

If you use Linux, it is always a good idea to check you code with a program called Valgrind. It helps to detect memory leaks and memory errors.

In addition, to read a character from stdin, I would use getchar(). It is simpler, faster. A small information about it in a Stack Overflow answer here.

And the last note, if you use a function that returns nothing, you don't need to write return in the end of the function.

Dynamic string allocation in c

1 Answers1