Dynamic array prints garbage after adding elements

Question

I've looked at my code for hours now and I just can't figure out why it doesn't work. This program creates a shopping list using a dynamic array. I can add new elements just fine. I can also save and read elements from a file. My problem is that whenever I read from a file and then add a new element my array gets messed up and my print function prints garbage. What it should do is that whenever I load a file or add a new element it should add it to the end of the list always. Here is my code:

typedef struct ShoppingList {
    int id;
    char name[100];
    int amount;
    char unit[10];
}ShoppingList;

void PrintShoppingList( ShoppingList *list, int *itemsLoaded )
{
    if ( *itemsLoaded == 0 ) {
        printf( "Inköpslistan är tom.\n" );
        return;
    }
    for ( int i = 0; i<*itemsLoaded; i++ ) {
        printf( "%d\t %s\t\t %d\t %s\n", list[i].id, list[i].name, list[i].amount, list[i].unit );
    }
}

void AddNewItem( ShoppingList *list, int *itemsLoaded ) {
    ShoppingList *temp;
    printf( "Namn på vara: " );
    scanf_s( "%s", list[*itemsLoaded].name, sizeof( list[*itemsLoaded].name ) );
    printf( "Antal: " );
    while ( scanf_s( "%d", &list[*itemsLoaded].amount ) != 1 ) {
        scanf_s( "%*s" );
        printf( "Antal: " );
    }
    printf( "Enhet: " );
    scanf_s( "%s", list[*itemsLoaded].unit, sizeof( list[*itemsLoaded].unit ) );
    list[*itemsLoaded].id = *itemsLoaded;
    temp = (ShoppingList*)realloc( list, ( *itemsLoaded + 2 ) * sizeof( ShoppingList ) );
    if ( temp != NULL ) {
        list = temp;
        *itemsLoaded = *itemsLoaded + 1;
        printf( "Vara lades till.\n" );
    } else {
        //free( list );
        printf( "Kunde inte allokera minne.\n" );
    }
}

void SaveShoppingListToFile( ShoppingList *list, int *itemsLoaded ) {
    if ( *itemsLoaded == 0 ) {
        printf( "Inköpslistan är tom. Kan ej spara.\n" );
        return;
    }
    char filename[20];
    int i;
    printf( "Spara fil som: " );
    scanf_s( "%s", filename, sizeof(filename) );
    FILE *fp;
    fopen_s( &fp, filename, "w" );
    if ( fp )
    {
        fprintf( fp, "%d", *itemsLoaded );
        for ( i = 0; i<*itemsLoaded; i++ ) {
            fprintf( fp, "\n%s\n%d\n%s", list[i].name, list[i].amount, list[i].unit );
        }
        fclose( fp );
        printf( "Fil sparad.\n" );
    } else {
        printf( "Kunde ej spara filen.\n" );
    }
}

void LoadShoppingListFromFile( ShoppingList *list, int *itemsLoaded ) {
    char filename[20];
    int nEntries = 0;
    ShoppingList *temp;
    printf( "Läs in fil: " );
    scanf_s( "%s", filename, sizeof( filename ) );
    FILE *fp;
    fopen_s( &fp, filename, "r" );
    if ( fp )
    {
        fscanf_s( fp, "%d", &nEntries );
        for (int i = 0; i<nEntries; i++ ) {
            fscanf_s(fp, "%s", list[*itemsLoaded].name, sizeof( list[*itemsLoaded].name ) );
            fscanf_s(fp, "%d", &list[*itemsLoaded].amount );
            fscanf_s(fp, "%s", list[*itemsLoaded].unit, sizeof( list[*itemsLoaded].unit ) );
            list[*itemsLoaded].id = *itemsLoaded;
            temp = (ShoppingList*)realloc( list, ( *itemsLoaded + 2 ) * sizeof( ShoppingList ) );
            if ( temp != NULL ) {
                list = temp;
                *itemsLoaded = *itemsLoaded + 1;
            } else {
                //free( list );
                printf( "Kunde inte allokera minne.\n" );
            }
        }
        fclose( fp );
        printf( "Fil inläst.\n" );
    } else {
        printf( "Kunde ej läsa filen.\n" );
    }
}

void Menu() {
    int menu = 0, *itemsLoaded, index = 0;
    itemsLoaded = &index;
    ShoppingList *list = NULL;
    list = (ShoppingList*)malloc( sizeof( ShoppingList ) );
    if ( list != NULL ) {
        do
        {
            system( "CLS" );
            menu = 0;
            printf( "%d\n", *itemsLoaded );
            printf( "Meny\n 1 - Lägg till en vara till inköpslistan\n 2 - Skriv ut inköpslistan\n 3 - Skriv inköpslistan till fil\n 4 - Läs in inköpslista från fil\n 5 - Ändra vara\n 6 - Ta bort vara\n 7 - Avsluta\nAnge Val: " );
            while ( scanf_s( "%d", &menu, sizeof( int ) ) != 1 ) {
                scanf_s( "%*s" );
                printf( "\nFelaktigt val. Försök igen.\n" );
                printf( "Ange Val: " );
            }
            if ( menu < 7 ) {
                switch ( menu ) {
                    case 1:
                        AddNewItem( list, itemsLoaded );
                        PrintShoppingList( list, itemsLoaded );
                        break;
                    case 2:
                        PrintShoppingList( list, itemsLoaded );
                        break;
                    case 3:
                        SaveShoppingListToFile( list, itemsLoaded );
                        break;
                    case 4:
                        LoadShoppingListFromFile( list, itemsLoaded );
                        PrintShoppingList( list, itemsLoaded );
                        break;
                }
            } else if ( menu > 7 || menu < 1 ) {
                printf( "Felaktigt val. Försök igen." );
            }
            system( "pause" );
        } while ( menu != 7 );
    } else {
        printf( "Kunde inte allokera minne." );
    }
    free( list );
}

int main() {
    Menu(); 
    return 0;
}

Try to use your debugger! Many modern IDEs allow you to run code line by line, and you will see where exactly the error occurs. If you still don't know where the problem is, please make your code smaller and post only relevant parts. No one wants to read whole 200 lines of code. — Ivan Smirnov, Oct 31 '16 at 21:40
As a matter of style and ease of use, I have found it better to have `numItems = *itemsLoaded` at the beginning of each function and `*itemsLoaded = numItems` just before the return. That helps the processing. Also print `numItems` before you output your list to ensure that you are updating it correctly. — sabbahillel, Oct 31 '16 at 21:51
I've used my debugger and it's after my system("pause"); statement in the menu where the array breaks. Everytime I've added an item or read from the file I print the list and it's fine however when it goes past that system pause it breaks and when I try to add or read it crashes and printing I get garbage. — FaderKerm, Oct 31 '16 at 22:00
When you do `list = temp;` with the realloc'd pointer, that only affects the `list` parameter local to the function. It does not change the pointer in the calling function. — Bo Persson, Oct 31 '16 at 22:22
Possible duplicate of [Initializing a pointer in a separate function in C](http://stackoverflow.com/questions/2486235/initializing-a-pointer-in-a-separate-function-in-c) — Bo Persson, Oct 31 '16 at 22:23

chqrlie · Accepted Answer · 2016-10-31T22:39:41.487

Function void AddNewItem( ShoppingList *list, int *itemsLoaded ) { reallocates list when the array is full but the pointer to the newly allocated array is never passed back to the caller which still uses the previous value. This invokes undefined behavior.

You should pass the address of the pointer so the function can update the caller's value.

int AddNewItem(ShoppingList **listp, int *itemsLoaded) {
    ShoppingList *list = *listp;
    printf( "Namn på vara: " );
    scanf_s( "%s", list[*itemsLoaded].name, sizeof( list[*itemsLoaded].name ) );
    printf( "Antal: " );
    while ( scanf_s( "%d", &list[*itemsLoaded].amount ) != 1 ) {
        scanf_s( "%*s" );
        printf( "Antal: " );
    }
    printf( "Enhet: " );
    scanf_s( "%s", list[*itemsLoaded].unit, sizeof( list[*itemsLoaded].unit ) );
    list[*itemsLoaded].id = *itemsLoaded;
    list = (ShoppingList*)realloc( list, ( *itemsLoaded + 2 ) * sizeof( ShoppingList ) );
    if ( list != NULL ) {
        *listp = list;
        *itemsLoaded = *itemsLoaded + 1;
        printf( "Vara lades till.\n" );
        return 1; // success
    } else {
        //free( list );
        printf( "Kunde inte allokera minne.\n" );
        return 0; // failure
    }
}

Call this function from main() as AddNewItem( &list, itemsLoaded );

Note that it would actually be simpler to reallocate the array before parsing an extra item. The initial pointer could be NULL for this approach.

The function LoadShoppingListFromFile has the same problem.

This works except I needed to change ShoppingList **list = *listp; to ShoppingList *list = *listp; Awesome thanks for the help! — FaderKerm, Oct 31 '16 at 22:38

Dynamic array prints garbage after adding elements

1 Answers1