0

I need to put the names separated by commas from the text into struct that expands dynamically, but I am prohibited from using realloc ().I'm getting a core dumped error in this code. What is the error in this code?

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

struct movie{

    double budget;
    int genre;
    char* name;
    double score;
    int year;
};


void recorder_function(struct movie *movies){

    FILE*fp;
    fp=fopen("Movies.txt","r");
    struct movie *p;
    int i,n=0;
    char line[1000];
    char temp_budget[50];
    char temp_name[50];
    char temp_genre[50];
    
    while (!feof(fp)) {
    
        fgets(line,1000,fp);
        sscanf(line,"%50[^,],%50[^,],%50[^,]",temp_budget,temp_genre,temp_name);
        
//I open the fields in this section

        movies=(struct movie *)calloc(n+1,sizeof(struct movie));
        p=(struct movie *)calloc(n+1,sizeof(struct movie));
        
        p[n].name=(char*)malloc(sizeof(char)*(strlen(temp_name)+1));
        movies[n].name=(char*)malloc(sizeof(char)*(strlen(temp_name)+1));

        for(i=0;i<n;i++)
        movies[i]=p[i];

        strcpy(movies[n].name,temp_name);

        free(p);
            
        p=movies;

        n++;
        
    }

    for(i=0;i<n;i++)
        printf("%s\n",movies[i].name);
        

}

int main(){

    int choice;
    struct movie *movies;
    recorder_function(movies);
}
   
Clifford
  • 88,407
  • 13
  • 85
  • 165
Rousso
  • 3
  • 1
  • 1
    Rousso, Who or what text suggested using `while (!feof(fp)) {`? – chux - Reinstate Monica May 17 '21 at 16:09
  • 2
    https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong – William Pursell May 17 '21 at 16:09
  • With scanf, the field width should be at most 1 less than the size of the buffer. So `char temp_budget[50];` goes with `sscanf("%49[^,]", ...)` – William Pursell May 17 '21 at 16:10
  • 1
    If you're not allowed to use `realloc()`, you have to read the file twice. First to get the number lines so you can allocate the `movies` array the proper size. Then the second time to fill it in. – Barmar May 17 '21 at 16:13
  • @WilliamPursell I got it, but I gave the word size too much, normal 10 does not exceed 15 words The words in the txt – Rousso May 17 '21 at 16:14
  • 1
    Why does `recorder_function()` take `movies` as a parameter? It immediately overwrites it with `calloc()`. – Barmar May 17 '21 at 16:14
  • I see, you're trying to implement your own `realloc()` by calling `calloc()` and then copying from the previous one. – Barmar May 17 '21 at 16:16
  • yes but i get a core dumped error – Rousso May 17 '21 at 16:17
  • 1
    You should check if `fopen()` succeeded. – MikeCAT May 17 '21 at 16:20
  • when i print it sequentially i see that there is no problem reading the file but the last time i print i see that it was not saved to the movies struct and it gave an error – Rousso May 17 '21 at 16:22
  • 1
    It is not guaranteed what happens when `NULL` is passed for `%s` format specifier. [c - What is the behavior of printing NULL with printf's %s specifier? - Stack Overflow](https://stackoverflow.com/questions/11589342/what-is-the-behavior-of-printing-null-with-printfs-s-specifier) – MikeCAT May 17 '21 at 16:23
  • 2
    Why does it allocate memory for both `movies` and `p` in the loop? – Ian Abbott May 17 '21 at 16:28
  • p[n].name should also be freed? might be memory leak – Beyonder May 17 '21 at 16:29
  • BTW: this is not related to your problem, but the casts before `malloc` and friends are useless. `= (char*)malloc(...` -> `= malloc(...` etc. – Jabberwocky May 17 '21 at 16:30
  • To keep data while growing dynamically because I am prohibited from using realloc – Rousso May 17 '21 at 16:30

1 Answers1

1

It is a bad idea to overwrite the pointer movie by newly allocated clean buffer.

Instead of that, you should

  1. Allocate new buffer only for p.
  2. Put the new element to p[n]
  3. Put existing elements movie[0], ... , movie[n-1] to p[0], ... , p[n-1]
  4. Free the old buffer movie
  5. Assign the new buffer p to movie

Don't forget to initialize movie not to cause troubles at the first freeing.

Also while (!feof(fp)) is wrong and you should check if readings are successful after trying to read and before using what are read.

One more important point is that you should make sure that fopen() succeeded. Passing NULL, which fopen() returns on failure`, to other file manipulation functions may cause troubles.

Another point is that your arrays used for sscanf() outputs should have one more elements for the terminating null-character.

Yet another point is that casting results of malloc() family is considered as a bad practice.

Try this:

void recorder_function(struct movie *movies){

    FILE*fp;
    fp=fopen("Movies.txt","r");
    if(fp==NULL){
        fputs("file open error\n", stderr);
        return;
    }
    struct movie *p;
    int i,n=0;
    char line[1000];
    char temp_budget[51];
    char temp_name[51];
    char temp_genre[51];
    
    movies=NULL;
    
    while (fgets(line,1000,fp)) {
    
        sscanf(line,"%50[^,],%50[^,],%50[^,]",temp_budget,temp_genre,temp_name);
        
//I open the fields in this section

        p=calloc(n+1,sizeof(struct movie));
        
        p[n].name=malloc(sizeof(char)*(strlen(temp_name)+1));
        strcpy(p[n].name,temp_name);

        for(i=0;i<n;i++){
            p[i]=movies[i];
        }

        free(movies);

        movies=p;

        n++;
        
    }

    for(i=0;i<n;i++){
        printf("%s\n",movies[i].name);
    }

}

The next stage will be fixing the weird usage of the argument movie. Arguments in C are copies of what are passed, and modifications of arguments in callee functions won't affect what are passed in caller. Your choice is:

  • Remove the argument movies and convert that to a local variable.
  • Have recorder_function take a pointer to struct movie* (struct movie**) and have it use the pointer to modify what caller specifies. (You also have to change the statement to call the function to pass a pointer in this case)
MikeCAT
  • 73,922
  • 11
  • 45
  • 70
  • 1
    @Rousso If you are prohibited from using `realloc()` for academic purposes then this solution is fine. If it is prohibited for some technical reason, then be aware that this solution is _exactly_ how `realloc()` works and may therefore be prohibited too. – Clifford May 17 '21 at 17:11