0

I am working on a project and have hit a snag that I have spent hours trying to figure out. I'm fairly certain its very close to correct but obviously something is wrong in my malloc of the struct array. I'll post the code below so you can see it. The goal of this function set is to read in movie data saved in a file and put the data into a structure.

#include <stdio.h>
#include <stdlib.h>
#include "support.h"
#include "scanner.h"
#include <string.h>

int counter(char *movierecords)
{
    int count = 0;
    char *name;
    char *about;
    int date;
    int time;
    char *rate;
    char *ppl;
    char *dir;
    //printf("test");
    FILE *fp = fopen(movierecords, "r");
    //printf("gggg");
    name = readString(fp);
    while (!feof(fp))
    {
        //printf("test in loop");
        about = readString(fp);
        date = readInt(fp);
        time = readInt(fp);
        rate = readToken(fp);
        ppl = readString(fp);
        dir = readString(fp);
        //printf("test read");
        free(name);
        free(about);
        free(rate);
        free(ppl);
        free(dir);
        //printf("test free");
        count++;
        name = readString(fp);
    }
    //printf("test escape loop");
    fclose(fp);
    //printf("test file close");
    return count;
}

movie **readRecord(char *movierecords, int count) //mallocates space and reads data into an array of movie structures
{
    int x = 0;
    movie **data1;
    FILE *fp = fopen(movierecords, "r");
    data1 = malloc(sizeof(struct movie *) * (count + 1)); //mallocate space for the struct array movies1
    printf("another test");

    while (x < count + 1) //loop mallocates space for the string variables in movies1 struct for all movies
    {
        data1[x]->moviename = malloc(sizeof(char) * 1001);
        data1[x]->description = malloc(sizeof(char) * 2001);
        data1[x]->rating = malloc(sizeof(char) * 10);
        data1[x]->cast = malloc(sizeof(char) * 512);
        data1[x]->director = malloc(sizeof(char) * 30);
        x++;
    }

    printf("test point3\n"); x = 0;
    while (!feof(fp))
    {
        data1[x]->moviename = readString(fp);
        data1[x]->description = readString(fp);
        data1[x]->year = readInt(fp);
        data1[x]->length = readInt(fp);
        data1[x]->rating = readToken(fp);
        data1[x]->cast = readString(fp);
        data1[x]->director = readString(fp);
        x++;
    }

    printf("test point4\n");
    fclose(fp);
    return data1;
}

Header file:

typedef struct entry
{
    char *moviename;
    char *description;
    int year;
    int length;
    char *rating;
    char *cast;
    char *director;
} movie;

int counter(char *movierecords); 
movie **readRecord(char *movierecords, int count);

Main:

#include <stdio.h>
#include <stdlib.h>
#include "support.h"
#include <string.h>
int main (int argc, char **argv)
{
    int count = 0;
    printf("%d", argc);
    movie **data1;
    count = counter(argv[1]);
    printf("%d", count);
    printf("hello");
    data1 = readRecord(argv[1], count);

    return 0;
}
user4164058
  • 545
  • 1
  • 4
  • 10
  • 7
    For one, you're eventually going to consume all memory allowed by your process with a loop like `while(x<=count)` that has no modification means whatsoever for either `x` nor `count`. In other words, the loop will never break if `count > 0`. Consequently, it spins forever, allocating (then leaking) memory. Also, [`while(!feof(fp))` is wrong](http://stackoverflow.com/questions/5431941/while-feof-file-is-always-wrong). – WhozCraig Oct 21 '14 at 02:38
  • Even after changing those things I still get a segmentation fault – Nathan Romans Oct 21 '14 at 02:47
  • 4
    May as well throw this is as well. `count` is uninitialized in the ill-advised `counter()` function. I'm hesitate to say with confidence there are likely more issues lurking. This needs to be debugged, and you need to crank up your compiler warnings as high as you can and **fix** what is flagged as potential issues. – WhozCraig Oct 21 '14 at 02:49
  • we are required to have the counter function and must use vim to compile and run our code which makes for a rather difficult debugging environment – Nathan Romans Oct 21 '14 at 03:15
  • What did you launch *vim* from? A shell? – WhozCraig Oct 21 '14 at 03:16
  • I use Xcode to code so that I can have syntax highlighting and easier formatting – Nathan Romans Oct 21 '14 at 03:20
  • 2
    Xcode has a fine debugger integration (lldb). Use it. single-step through that code. There are other issues, most of it simple logic mistakes or omissions. It will take some time to groom this up to presentable. So don't rush it. – WhozCraig Oct 21 '14 at 03:22
  • I figured it did but can not find where to start the debug process – Nathan Romans Oct 21 '14 at 03:24
  • 3
    Ah. you only use Xcode for editing, you don't have an actual *project* created? Its simple. Just make a console OS X application project, then drop your code into the main source file. Once there, you get compilation, debugging, even profiling (not that you should care about that right now). The docs online for Xcode are pretty good. Hit up [apples website](http://developer.apple.com). – WhozCraig Oct 21 '14 at 03:26
  • thanks illl try that and can hopefully figure this out – Nathan Romans Oct 21 '14 at 03:27
  • 2
    `feof(fp)` will not be true until _after_ a read has failed. – chux - Reinstate Monica Oct 21 '14 at 03:33
  • Okay so i found where the problem is occurring it is when i malloc the arrays inside the struct... if someone could help me find the error it would be much apreciated as i am new to programing and this is the first time i have worked with arrays in a structure – Nathan Romans Oct 21 '14 at 13:37
  • All the calls to malloc() need to be immediately followed by a check of the returned value to assure the malloc was successful. – user3629249 Oct 21 '14 at 15:56
  • the function counter() makes the assumption that if the first field of a record is available, that all the following fields will be available. This could easily be incorrect. Also, the returned value from readString(), etc can be NULL, so that needs to be checked after each invocation. – user3629249 Oct 21 '14 at 16:00
  • these kinds of statements: data1[x]->moviename = readString(fp); will not copy the actual string. They only save the ptr returned from readString() and similar functions. suggest using: strcpy( data1[x], readString(fp) ); Also, per the counter() function, the functions like readString() malloc an area that needs to be free'd, so (to avoid memory leaks) need to add a call to free() after each of the read...() functions. This means the returned pointers need to be saved in a temp var so they can be used to free the memory – user3629249 Oct 21 '14 at 16:09
  • at the end of main(), all the memory allocated for data1[] and its' sub fields needs to be free'd to avoid memory leaks. – user3629249 Oct 21 '14 at 16:12

1 Answers1

1

You have to allocate memory for this "data1[x]" because your malloc(double pointer) allocate memory for array of your records(single pointer). Fot the single pointer data[x] you have to allocate memory

Saravanan
  • 153
  • 6