Search code examples
carraysdynamicmallocrealloc

Dynamic array prints garbage after adding elements


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;
}

Solution

  • 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.