-1

Okay, I'm very new to C and need an explanation as to why I'm getting this error:

"variable 'newFilm' has initializer but incomplete type"

The task is to create a struct called film. Then pass the data from a .txt file into that struct and create a linked list of structs representing all of the data in the .txt

The problem seems to be that the compiler is missing the point where I allocate memory for the struct newFilm which I believe is being done correctly

Code in main file:

char* t = (char*)malloc(sizeof(char*));
int y;
char* r = (char*)malloc(sizeof(char*));
char* g = (char*)malloc(sizeof(char*));
int rt;
double s;

List* list = newList();

//read pReadFile
char input[256];
//read characters from file being pointed at, and store into input
    while( fgets( input, 256, pReadFile )) {
        //scan each line with each variable separated by a comma
        fscanf(pReadFile,"%s %d %s %s %d %d\n", t,y,r,g,rt,s);    
        struct Film newFilm = createFilm(t,y,r,g,rt,s); //ERROR OCCURS HERE
        addToList(list, newFilm); 
}

printList(list, pWriteFile);

Here is the createFilm function from film.c source file:

Film *createFilm(char *title, int year, char *rating,  
                  char *genre, int runtime, double score){

   Film *newFilm = (Film*)malloc(sizeof(Film));   
   // n.b. error checking to be added - to be added

    title = (char*)malloc(sizeof(title));
    newFilm->title = title;


    newFilm->year = year;

    rating = (char*)malloc(sizeof(rating));
    newFilm->rating = rating;

    genre = (char*)malloc(sizeof(genre));
    newFilm->genre = genre;


    newFilm->runtime = runtime;


    newFilm->score = score;



    return newFilm;
}

While I don't think there is anything wrong with the addToList function I thought I'd keep it so you have better context (in database.h file):

void addToList(List* list, struct Film* film){

    Node *node = (Node*)malloc(sizeof(Node));

    //Generates an error message and the program terminates if 
    //insufficient memory is available.
    if (node == NULL){

        fprintf(stderr, "Error: Unable to allocate memory in list_add()\n");

        exit(EXIT_FAILURE);
    }

    //appends film to tail of linked list
    node->film = film;
    node->next = NULL;

    if (list->last == NULL){
        list->first = list->last = node;
    }
    else{
        list->last = list->last->next = node;
    }
}

Thanks in advance :)

Vinny Mann
  • 1
  • 1
  • 2
  • 1
    Welcome to Stack Overflow! [Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). – Sourav Ghosh Nov 14 '16 at 21:00
  • `Here is the createFilm function from film.h header file`...wait..what? – Sourav Ghosh Nov 14 '16 at 21:01
  • Sorry, not header file. got confused. – Vinny Mann Nov 14 '16 at 21:03
  • Let us assume the sizeof a pointer is 4. `char* t = (char*)malloc(sizeof(char*));` is strange. Why allocate for `t`, which points to a `char` array, 4 bytes? If code needs an array of 100 `char`, use `char* t = malloc(100);` – chux - Reinstate Monica Nov 14 '16 at 21:55

1 Answers1

3

You are missing the declaration of the struct. With struct Film; you can create as many struct Film * pointers as you would like, because the compiler can figure out how big a pointer to a film will have to be (big enough to point to a struct).

However, since all you have is that Film is a struct (not what the struct is, or how big it is) you can't actually create a struct Film variable, since the compiler can't know how much space to allocate for that. There are two fixes for this:

  1. Make the entire struct visible.

This likely involves moving the struct definition (not just declaration) to the header file. IE:

// old film.h
struct Film;

// new film.h
struct Film {
    int with;
    int all;
    int of;
    int the;
    int things;
    int it;
    int needs;
};
  1. Make the entire struct opaque, and use opaque accesses.

This means that you never actually create struct Film anywhere in the code that uses it. Instead you write functions to create/destroy a film pointer and to access/modify each element.

Typically option 2 is more extensible (since changing the struct doesn't affect the code) but option 1 is easier.

LambdaBeta
  • 1,479
  • 1
  • 13
  • 25