digest
digest

Reputation: 19

How to print an array of structs, recursively?

I have this array with 100 movies (struct Movie, I got the data from a previously loaded simply linked list), and for some reason when I try to print the content (in this case, just the title of the movie) it only shows 45 movies out of the 100. And then the program stops. It's part of a bigger code, but the rest works fine so I'm gonna post the code that I have problems with.

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

#define SIZE 100

// List of movies
struct Node{
    Movie movie;
    struct Node *next; 
};

// Pointer to the first element of the list
typedef struct{
struct Nodo *head;
}List;

typedef struct{
    int id;
    char title[100];
    char director[100];
    char genre[100];
    int likes;
    int number_of_voters;
    float rating;
    int year;
    int cost;
    char color[100]; // "Color" or "BW"
}Movie;

typedef struct{
     Movie movies[SIZE];
     int cant; // Number of structs loaded
}Array;

void Initialize(List *l);
void FromListToArray(List l, Array *arr);
void PrintArray(Array arr, int length);

int main(){
    List list;  // Previously loaded with movies, which I copied from a txt file
    Array a;
    Initialize(&list);
    FromListToArray(list, &a);
    // Array previously loaded with 100 movies (index 0 to 99), so a.cant = 100
    int length = a.cant - 1;
    PrintArray(a, length)
    return 0;
}

void PrintArray(Array arr, int length){
    if (lenght < 0){    // Base
        return;
    }
    // Inductive
    printf("%s\n", arr.movies[length].title);
    PrintArray(arr, sup-1);
}

/* Initializes a simply linked list */
void Initialize(List *l){
    (l->head) = NULL;
}

/* Copies the data of "colored" movies (there are black and white movies too in the list) from a simply linked list into an array */
void FromListToArray(List l, Array *arr){
    int i = 0;
    arr->cant = 0;
    struct Node *p;
    p = l.head;
    while ((p->next != NULL) && (arr->cant < SIZE)){
        if (strcmp(p->movie.color, "Color\n")==0){
            // Copy the content of the movie from the list into the array
            arr->movies[i].id = p->movie.id;
            strcpy(arr->movies[i].title, p->movie.title);
            strcpy(arr->movies[i].director, p->movie.director);
            strcpy(arr->movies[i].genre, p->movie.genre);
            arr->movies[i].likes = p->movie.likes;
            arr->movies[i].number_of_voters = p->movie.number_of_voters;
            arr->movies[i].rating = p->movie.rating;
            arr->movies[i].year = p->movie.year;
            arr->movies[i].cost = p->movie.cost;
            strcpy(arr->movies[i].color, p->movies.color);
            arr->cant++;
            i++;
        }
        p = p->next;
    }
    printf("Movies in the array ::: %d\n", arr->cant);
}

Upvotes: 0

Views: 580

Answers (1)

grek40
grek40

Reputation: 13458

Since the issue was already resolved in comments, this is a quick wrap-up of the problem and solution.

Analysis

You created a recursive function that should reach a recursion depth of 100. Instead, at some 40 recursive calls, the program crashes with a stackoverflow.

Since the stack should typically provide at least 1mb of space, you can assume that something is occupying much more stack space per recursive call than what you assumed. Stack is mostly occupied by local variables and function parameters.

In your case, PrintArray doesn't define any local variables within the function, so the function parameters are the most likely culprit.

The Array parameter reserves stack space according to the struct size for each recursive call, since the parameter is taken by value, rather than by reference (pointer).

The Array struct size is quite large, since it contains an array of 100 elements, where each element consists of a struct Movie, which in turn contains 4 char arrays of size 100 each, among other properties. So, ignoring the other properties and all alignment extra space, the size of Array can be abstracted as 100 elements * 4 arrays * 100 characters each => 40000 char.

So, for each recursive call, more than 40000 byte are copied to the next stack segment. Inflating this by 40 recursive calls gives 40000 * 40 = 1600000 bytes which is 1.5mb plus (remember, I'm being very inaccurate here...) in required stack size.

How to fix

Typically, this kind of task would be performed iteratively in a loop - you mentioned you can't do that due to specific constraints of your task.

As mentioned in the comments, the problem goes away by replacing the Array parameter by a pointer type Array* and changing the usage accordingly. Since the function only ever reads from the array, I would recommend making the parameter const Array*.

Even with the pointer, stack space is reserved for each parameter for each recursive call. But now the amount of required space is determined by the size of the pointer (4-8 bytes typically), the length parameter and some internal values like the return address. So the arr parameter space requirement decreased by a factor of at least 5000 (but the overall stack usage decreased by a bit lower factor).

The decreased size requirement means, you should now be able to print your list of 100 items, but it also means that you are likely to run into the same problem again, if your list contains a few thousand instead of a few hundred items.

Side Notes (might be irrelevant)

Your length parameter doesn't do what the name promises. It is not used as a length information. Instead it is used as a last accessable index. Variable names should be consistent with their content / usage.

It seems you print your array back-to-front.

In function FromListToArray, you loop on the condition p->next != NULL. This looks suspicious for two reasons:

  1. if the list is empty and p is NULL you probably run into trouble
  2. the last element p is not processed in the loop, just because it doesn't have a successor.

Upvotes: 1

Related Questions