-1

I have a school project where I need to read .ppm file in C and store it in a struct to be used for later tasks. Once I get the first line and assign it to a struct variable, I get errors if I try to navigate through the file again. Here is my code:

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

typedef struct {
    int r, g, b;
} pixels;

typedef struct {
    int width, height, maxColors;
    char *format; //PPM file format
    char *comments;
    pixels *pixels;
} PPM;

PPM *getPPM(FILE * f) {

    // Check if the file is valid. If not then exit the program
    if(f == NULL) {
        perror("Cannot open file: ");
        exit(1);
    }

    PPM *pic = malloc(sizeof(PPM));
    // If memory allocation fails, exit the program.    
    if(!pic) {
        perror("Unable to allocate memory for structure");
        exit(1);
    }

    // Store the first line of file into the structure
    char *fileFormat;

    if(fgets(fileFormat, sizeof(fileFormat), f) != NULL) {
        // If it is a valid .ppm format, then store it in the structure
        if(fileFormat[0] == 'P') 
            pic->format = fileFormat;
    } else {
        perror("Unable to read line from the input file");
        exit(1);
    }

//Errors start here
    int c = getc(f);
    while(c != EOF)
        c = getc(f);

/*
    char *comments;

    if(fgets(comments, sizeof(comments), f) != NULL) {
        printf("%s\n", comments);
    } else {
        perror("Unable to read line from the input file");
        exit(1);
    }
*/
    fclose(f);
    return pic; 
}



int main(int argc, char **argv) {

    PPM *p = getPPM(fopen(argv[1], "r"));
    printf(" PPM format = %s",p->format);
    return 0;
}

I have tried getting individual characters from the file. I have tried reading the whole line using fgets like I did in the previous step (for fileFormat), but every time it gives out segment fault. I have tried looking at other examples but I cannot figure out the problem. I have been at it for hours, so any help would be greatly appreciated!

Could the problem with the way the memory is allocated? Or do I need to give some sort of pointer to the file when I try to read a new line? I tried finding an answer in man pages but I couldn't figure anything out.

P.S. the while(c != EOF) { c = getc(f); } and the next commented step was just to see if it was working or not. I want to put all the information from the .ppm file into the PPM struct.

Naeem Khan
  • 950
  • 4
  • 13
  • 34
  • 2
    `char *fileFormat; fgets(fileFormat, sizeof(fileFormat), f)` - where is it going to store whatever it read? – Eugene Sh. Feb 13 '19 at 19:02
  • I want to store it in pic->comments (PPM->comments). But when I tried to store it directly it gave me errors, so I stored the line in a string and put that string in the structure. – Naeem Khan Feb 13 '19 at 19:05
  • 2
    *"... so I stored the line in a string..."* - no , you didn't. You stored the line at an indeterminate address hosted by an uninitialized pointer, and in so doing, invoke undefined behavior. – WhozCraig Feb 13 '19 at 19:07
  • I am very new to C and it is my first time dealing with a low-level language, so I do not understand what you mean by that. Would you please explain a bit more clearly? – Naeem Khan Feb 13 '19 at 19:10
  • 1
    Small tip: Use the -Wall flags when compiling with gcc to enable all Warnings. And if you get a warning, try to understand, google it if you can't, and fix it. – Thomas Leyk Feb 13 '19 at 19:22
  • Thank you. I will try that from now on. – Naeem Khan Feb 13 '19 at 19:28
  • This is not a proper [mcve]. Especially the program will crash before the `getc` loop and al, so in the course of preparing your question you should've found out that they're not relevant to the question. – Antti Haapala -- Слава Україні Feb 13 '19 at 20:31
  • I thought that problem was with struct and its memory allocation. That is why I included all that. I don't understand why did you downvote this post when the question was already answered? The problem I thought was not the problem, and the reason why I couldn't find anything about it. That's the reason why I asked the question over here. – Naeem Khan Feb 14 '19 at 14:11

1 Answers1

2

You're reading into an uninitialized pointer, so this is going to crash. You need a buffer:

char *fileFormat = malloc(SIZE_OF_FILE_FORMAT);

Additionally sizeof(fileFormat) returns the size of the pointer which in this case is not what you want. You need the size of the buffer the pointer points to.

tadman
  • 208,517
  • 23
  • 234
  • 262
  • @EugeneSh. Added a remark about that. This is probably just the tip of the iceberg. – tadman Feb 13 '19 at 19:05
  • But I don't understand that why does it manage it populate work by populating the struct. I mean the pic->format = "P3" in this case works as I expected it to. Would you please clarify what am I missing? – Naeem Khan Feb 13 '19 at 19:12
  • 1
    When you're using character buffers you **must allocate them first**. This can be done like `char buffer[BUFFER_SIZE]` or `char* buffer = malloc(BUFFER_SIZE)` depending if you want dynamic allocation or not. In your case you stuffed a completely uninitialized, unallocated pointer into a function call. That's going to crash for sure. – tadman Feb 13 '19 at 19:13
  • Oh! I get it. That makes my understanding of malloc also a LOT more clearer. Thank you so much! – Naeem Khan Feb 13 '19 at 19:16
  • 1
    Getting the hang of pointers can be a bit tricky, so if you have problems like this, step through your code in the debugger to see what values your pointers have. Often they'll be `NULL` in which case you're in for a bad time if you use them. When you discover an accidentally uninitialized pointer, look for a way to allocate it with an appropriate size. – tadman Feb 13 '19 at 19:17
  • 1
    Thank you. I will keep that in mind from now on. – Naeem Khan Feb 13 '19 at 19:29