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
– Ivan Smirnov, Commented Oct 31, 2016 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
– sabbahillel, Commented Oct 31, 2016 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
– FaderKerm, Commented Oct 31, 2016 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
– Bo Persson, Commented Oct 31, 2016 at 22:22
Possible duplicate of Initializing a pointer in a separate function in C — Bo Persson
– Bo Persson, Commented Oct 31, 2016 at 22:23

chqrlie · Accepted Answer · 2016-10-31 22:39:41Z

1

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.

edited Oct 31, 2016 at 22:39

answered Oct 31, 2016 at 22:25

chqrlie

152k12 gold badges145 silver badges231 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

FaderKerm Over a year ago

This works except I needed to change ShoppingList **list = *listp; to ShoppingList *list = *listp; Awesome thanks for the help!

Collectives™ on Stack Overflow

Dynamic array prints garbage after adding elements

1 Answer 1

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related