Using fgets to read through file in C

Question

I am trying to read through the file given then tokenize it. The only problem im having is fgets.The file open recieves no errors. I have seen this elsewhere on the site however no matter how i set this up including setting fileLine to a set amount like (char fileline [200]) i get a segmentation fault. Thanks in advance for any help.

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

 int main(int argc, char *argv[]){
    char *fileName = "0";
    char *tokenize, *savePtr;
    struct Record *database= malloc(sizeof(database[0]));
    int recordNum =0;
    char *fileLine = malloc(sizeof(char *));//have replaced with fileline[200] still didnt work
    FILE *fd = open(fileName,O_RDWR);
    if(fd< 0){
        perror("ERROR OPENING FILE");
    }

    while(fgets(fileLine,200,fd) !=NULL){
        printf("%s\n", fileLine);
        tokenize = strtok_r(fileLine,",",&savePtr);
        while(tokenize != NULL){
         //TOKENIZING into a struct
        }
}

you're telling fgets to fetch 200 chars into a string that's been malloced to be sizeof(*char), e.g. 200 bytes into a **4 BYTE** variable (e.g. size of a pointer to a string). you need `malloc(200 * sizeof(char))` — Marc B, Apr 16 '15 at 18:25
@MarcB even when ive used what you suggested or when ive made it a set array of 200(char fileLine[200];) i still get a segmentation fault. — justin, Apr 16 '15 at 18:30
@iharob the second while loop is not the issue. i have run this without fgets and it tokenizes properly with a hardcoded fileLine. The issue is at the fgets because i never make it to the printf statement afterword. — justin, Apr 16 '15 at 18:35
@justin that's just wrong, you can't be sure. Undefined behavior does not always happen the same way, it's undefined. — Iharob Al Asimi, Apr 16 '15 at 18:36

gsamaras · Accepted Answer · 2023-04-19T09:17:54.453

8

Why use open() with FILE? Use fopen() instead.

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

int main(int argc, char *argv[]) {
  char *fileName = "test.txt";
  char *tokenize, *savePtr;
  char fileLine[200] = {0}; // init this to be NULL terminated
  FILE *fd = fopen(fileName, "r");
  if (fd == 0) { // error check, equal to 0 as iharob said, not less than 0
    perror("ERROR OPENING FILE");
    return -1;
  }

  while (fgets(fileLine, 200, fd) != NULL) {
    printf("%s\n", fileLine);
    tokenize = strtok_r(fileLine, ",", &savePtr);
    while (tokenize != NULL) {
      tokenize = strtok_r(NULL, ",", &savePtr); // do not forget to pass NULL
      //TOKENIZING into a struct
    }
  }
  fclose(fd);

  return 0;
}

As Weather Vane said, fd < 0 would work if you used open(). However, with fopen(), you should check to see if the pointer is NULL, ecquivalently fd == 0.

A comparison between this functions that open a file can be found in:

The way I have it in mind is that fopen() is of higher level.

edited Apr 19 '23 at 09:17

answered Apr 16 '15 at 18:33

gsamaras

71,951
46
188
305

Wow, I didn't see that ... note that `fd < 0` would be wrong. – Iharob Al Asimi Apr 16 '15 at 18:35
Note: OP is using `open`, not `fopen`. So `fd < 0` would have been OK, but he wrote `*fd = open()` – Weather Vane Apr 16 '15 at 18:44
@WeatherVane of course, but now we used `fopen()`. However, I should clear it up, editing.. – gsamaras Apr 16 '15 at 18:45
1

Agreed @AlterMann, edited, thanks! I could also get rid of `stdlib` too I guess. – gsamaras Apr 16 '15 at 18:54
Note: The ` = {0}` in `char fileLine[200] = {0};` is not needed for this code. Not a _bad_ thing to do either. – chux - Reinstate Monica Apr 16 '15 at 19:59
@chux if nothing is read by `fgets()`, then `fileLine` will be left unchanged and I wouldn't like to see it uninitialized later, but I see your point. – gsamaras Apr 16 '15 at 20:31
"if nothing is read by fgets(), then fileLine will be left unchanged" does not always agree with the C spec for `fgets()`. "If a read error occurs during the operation, the array contents are indeterminate and a null pointer is returned." §7.21.7.2 – chux - Reinstate Monica Apr 16 '15 at 21:16
I am not talking about an error @chux. From the ref, "If this happens before any characters could be read, the pointer returned is a null pointer (and the contents of str remain unchanged)." – gsamaras Apr 16 '15 at 21:17
@G. Samaras I take it your reference is not from a C spec. – chux - Reinstate Monica Apr 16 '15 at 21:21
@chux I am copy pasted from this: http://www.cplusplus.com/reference/cstdio/fgets/ – gsamaras Apr 16 '15 at 21:22
I see the quote also says "If a read error occurs, the error indicator (ferror) is set and a null pointer is also returned (but the contents pointed by str may have changed)." Even though you might not be talking about an error, I see no way to prevent `fgets()` from responding to errors with an indeterminate buffer. – chux - Reinstate Monica Apr 16 '15 at 21:38
1

Me neither @chux, thanks for the nice discussion. :) – gsamaras Apr 16 '15 at 21:40
1

Don't forget to call `fclose(fd);` afterwards. – scai Apr 18 '23 at 08:28
@scai tnx, answer updated, hope you agree now. – gsamaras Apr 19 '23 at 09:18

score 2 · Answer 2 · answered Apr 16 '15 at 18:30

This line

char *fileLine = malloc(sizeof(char *));

allocates memory for a char * type, 4 or 8 bytes (depending on the platform).

So when you do

fgets(fileLine,200,fd)

it expects there to be 200 bytes of memory available.

Try this:

char *fileLine = malloc(200);
if (fileLine == NULL) { ... }   // check for error

which will allocate the memory required.

score 1 · Answer 3 · answered Apr 16 '15 at 18:32

You are using open() instead of fopen().

You can't be sure that the file did open correctly because fopen() does not return an integer, but a pointer to a FILE * object, on failure it returns NULL, so the right codition is

FILE *file;

file = fopen(filename, "r");
if (file == NULL)
 {
    perror("fopen()");
    return -1;
 }

In your code, you still go and use fgets() even when the fopen() fails, you should abort the program in that case.

Also, malloc() takes the number of bytes as the size parameter, so if you want fgets() to be limited to read just count bytes, then malloc() should be

char  *buffer;
size_t count;

count  = 200; /* or a value obtained someway */
buffer = malloc(count);
if (buffer == NULL)
 {
    fclose(file);
    perror("malloc()");
    return -1;
 }

All the problems in your code would be pointed out by the compiler if you enable compilation warnings.

Using fgets to read through file in C

3 Answers3