Char pointer in array is not freeing

Question

I have an array of char pointers. This array have strings. When I want to free the allocated memory for one of the strings it works, but the string after the removed one turns into rubbish. This is my code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define MAX 3
void AddName(char *Names[],int *size)
{
    char str[100];
    int len;
    if (*size < MAX)
    {
        printf("Enter a string: ");
        scanf(" %s", str);
        len = strlen(str);
        Names[*size] = (char *)malloc((len+1)*sizeof(char));
        strcpy(Names[*size],str);
        *size = *size+1;
    }
    else printf("Full Memory!!\n");
}
void RemoveName(char *Names[],int *size)
{
    int loc;
    if (*size > 0)
    {
    printf("Starting from 0. Enter the name's location: ");
    scanf(" %d", &loc);
    free(Names[loc]);
    *size = *size-1;
    }else printf("ERROR\n");
}
void PrintNames(char *Names[],int size)
{
    int i;
    for (i = 0; i<size; i++)
    {
        printf(" %s\t", Names[i]);
    }
    printf("\n");
}
main()
{
    char *Names[MAX];
    int size = 0;
    int c;
    do
    {
        printf("=========================\n");
        printf("1- Add a new name.\n");
        printf("2- Delete an old name.\n");
        printf("3- Print names.\n");
        printf("4- Exit.\n");
        printf("=========================\n");
        printf("Enter your choice: ");
        scanf("%i", &c);
        printf("=========================\n");
        switch(c)
        {
            case 1: AddName(Names,&size);
            break;
            case 2: RemoveName(Names,&size);
            break;
            case 3: PrintNames(Names,size);
            break;
            case 4: printf("Good bye.\n");
            break;
            default: printf("ERROR: Bad input.\n");
        }
    }while(c != 4);
}

If I print the names in the array, it gives Carl Hello. If I try to print again after freeing Carl it gives ↨Y (it should print Hello).

Answer 1

In addition to the main problem identified by RADAR in their answer, there are several other issues:

1. Here:

   Names[*size] = (char *)malloc((len+1)*sizeof(char));
   

   don't cast the return from malloc(), and sizeof(char) is always one, so this should be:

   Names[*size] = malloc(len + 1);
   

2. You should check the return from malloc(), because it can fail and return NULL.

3. Here, as well as other places:

   printf("Enter a string: ");
   scanf(" %s", str);
   

   You can get a problem because the thing you're sending to printf() does not end with a newline. Interactive input and output is line-buffered by default in C, and on some systems you won't see that prompt until after you've entered your input. To avoid this, call fflush(stdout) if you want to make sure some output that does not end with a newline displays immediately.

4. main() returns an int, and if you don't want to use command line arguments, the proper declaration is int main(void). While main() is the one function returning a value where you don't have to explicitly return one, it's better form if you do.

5. This:

   scanf(" %s", str);
   

   will lead to Bad Times if the user enters more than 100 characters. You can cope with this with scanf() but using fgets() is better.

6. In general, this type of input strategy using scanf() is fragile, particularly if the user enters the wrong thing. Particularly if you're getting a lot of input, you'll do yourself a big favor by defining some convenience functions to get lines and integers using fgets(), and re-using those functions.

7. Your formatting is pretty bad in some spots.

Here's a modified version taking all of the above into account:

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

#define MAX 3
#define BUFFER_LEN 100

/*  Convenience function to get a line from
 *  standard input - caller must free         */

char * get_line(void)
{
    char str[BUFFER_LEN];
    char * newstr;
    int len;

    /*  Get line of input  */

    if ( !fgets(str, sizeof str, stdin) ) {
        fprintf(stderr, "ERROR: Couldn't get string!\n");
        exit(EXIT_FAILURE);
    }

    /*  Remove trailing newline, if present  */

    len = strlen(str);
    if ( len > 0 && str[len - 1] == '\n' ) {
        str[--len] = 0;
    }

    /*  Allocate memory and copy string  */

    if ( !(newstr = malloc(len + 1)) ) {
        perror("couldn't allocate memory");
        exit(EXIT_FAILURE);
    }

    strcpy(newstr, str);

    return newstr;
}

/*  Convenience function to get an integer from standard input  */

long get_long(void)
{
    char * str, *endptr;
    long n;

    str = get_line();
    n = strtol(str, &endptr, 0);
    if ( *endptr ) {
        fprintf(stderr, "ERROR: You didn't enter an integer!\n");
        exit(EXIT_FAILURE);
    }

    free(str);

    return n;
}

/*  Adds a name to a list  */

void AddName(char * Names[], int * size)
{
    if ( *size < MAX ) {
        printf("Enter a string: ");
        fflush(stdout);

        Names[*size] = get_line();
        *size += 1;
    }
    else {
        printf("ERROR: List full!\n");
    }
}

/*  Removes a name from the list  */

void RemoveName(char * Names[], int * size)
{
    if ( *size > 0 ) {
        int i;
        long loc;

        printf("Starting from 0, enter the name's location: ");
        fflush(stdout);

        /*  Free the desired name...  */

        loc = get_long();
        free(Names[loc]);

        /*  ...and move any names ahead of it back.  */

        for ( i = loc + 1; i < *size; ++i ) {
            Names[i - 1] = Names[i];
        }

        *size -= 1;
    }
    else {
        printf("ERROR: list empty!.\n");
    }
}

/*  Prints the names in the list  */

void PrintNames(char * Names[], int size)
{
    int i;
    for ( i = 0; i < size; ++i ) {
        printf(" %s\t", Names[i]);
    }
    putchar('\n');
}

/*  Main function  */

int main(void)
{
    char * Names[MAX];
    int size = 0;
    long c;

    do {
        printf("=========================\n");
        printf("1- Add a new name.\n");
        printf("2- Delete an old name.\n");
        printf("3- Print names.\n");
        printf("4- Exit.\n");
        printf("=========================\n");

        printf("Enter your choice: ");
        fflush(stdout);
        c = get_long();

        printf("=========================\n");

        switch( c ) {
            case 1:
                AddName(Names, &size);
                break;

            case 2:
                RemoveName(Names, &size);
                break;

            case 3: 
                PrintNames(Names, size);
                break;

            case 4:
                printf("Good bye.\n");
                break;

            default:
                printf("ERROR: Bad input.\n");
                break;
        }
    } while ( c != 4 );

    return 0;
}

Answer 2

Your input is Carl and Hello

Names[0] = Carl

Names[1] = Hello

Now you freed up the memory for Names[0]

When you print names, you are trying to print Names[0] which is already freed up, so you are getting garbage.

You need to move all the pointers to fill up the empty space

In this case on deletion of Names[0], you have only one entry so Have Names[0] point to memory at Names[1]

When you delete entry at i you need to adjust the pointers like below till the end of the list.

Names[i] -> Names[i+1]