[SOLVED] Getting a segfault. Can't find it.

jmgibson1981 · 03-13-2023, 12:22 AM

Narrowed it to this function via gdb. it segfaults when the 2nd for loop starts / is supposed to start.

Code:

void file_reverse_output(FILE * file, FILE * newfile, const char type)
{
  // declare and initialize count + buffers + switch for tempfile
  int lines = file_num_lines(file);
  char buf[lines + 1][256];
  char str[256];

  // load buffer with lines
  for (int i = lines; i >= 0; i--) {
    strcpy(buf[i],fgets(str, 256, file));
  }
  // segfault here before next loop

  // output buffer in reverse order to file or screen
  // type is (p)rintf or (f)printf
  for (int i = 0; i <= lines; i++) {
    if (type == 'p') {
      if (i == 0) {
        printf("%s\n", buf[i]);
      } else {
        printf("%s", buf[i]);
      }
    } else if (type == 'f') {
      if (i == 0) {
        fprintf(newfile, "%s\n", buf[i]);
      } else {
        fprintf(newfile, "%s", buf[i]);
      }
    }
  }

  // reset file to beginning
  rewind(file);
}

NevemTeve · 03-13-2023, 12:55 AM

(Let's hope you call `rewind` after `file_num_lines`.)
You have an off-by-one error. Also you didn't check the return value of `fgets`.

Code:

void file_reverse_output(FILE * file, FILE * newfile, const char type)
{
  // declare and initialize count + buffers + switch for tempfile
  int lines = file_num_lines(file);
  char buf[lines + 1][256];
  char str[256];

  // load buffer with lines
  for (int i = lines-1; i >= 0; i--) {
    char *p= fgets(str, 256, file);
    if (!p) {
        fprintf(stderr, "Ouch\n");
        exit(12);
    }
    strcpy(buf[i],p);
  }
  // segfault here before next loop

  // output buffer in reverse order to file or screen
  // type is (p)rintf or (f)printf
  for (int i = 0; i < lines; i++) {
    if (type == 'p') {
      if (i == 0) {
        printf("%s\n", buf[i]);
      } else {
        printf("%s", buf[i]);
      }
    } else if (type == 'f') {
      if (i == 0) {
        fprintf(newfile, "%s\n", buf[i]);
      } else {
        fprintf(newfile, "%s", buf[i]);
      }
    }
  }

  // reset file to beginning
  rewind(file);
}

[/code]

NevemTeve · 03-13-2023, 01:17 AM

Guess you are working with a file that misses the terminating NewLine character. That should be solved otherwise, e.g.

Code:

static int file_num_lines(FILE *file) {
    int c;
    int lastc= '\n';
    size_t ln= 0;

    while ((c= fgetc(file))!=EOF) {
        if (c=='\n') ++ln;
        lastc= c;
    }
    rewind(file);
    if (lastc!='\n') ++ln;
    return ln;
}

or saving a line:

Code:

static int file_num_lines(FILE *file) {
    int c;
    int lastc= '\n';
    size_t ln= 0;

    while ((c= fgetc(file))!=EOF) {
        if (lastc=='\n') ++ln;
        lastc= c;
    }
    rewind(file);
    return ln;
}

Also with fgets:

Code:

void file_reverse_output(FILE * file, FILE * newfile, const char type)
{
  // declare and initialize count + buffers + switch for tempfile
  int lines = file_num_lines(file);
  char buf[lines + 1][256];
  char str[256];
  int l;

  // load buffer with lines
  for (int i = lines-1; i >= 0; i--) {
    char *p= fgets(str, 256, file);
    if (!p) {
        fprintf(stderr, "Ouch\n");
        exit(12);
    }
    l= strlen(p);
    if (l>0 && p[l-1]=='\n') p[--l]= '\0';
    strcpy(buf[i],p);
  }

  // output buffer in reverse order to file or screen
  // type is (p)rintf or (f)printf
  for (int i = 0; i < lines; i++) {
    if (type == 'p') {
        printf("%s\n", buf[i]);
    } else if (type == 'f') {
        fprintf(newfile, "%s\n", buf[i]);
    }
  }

  // reset file to beginning
  rewind(file);
}

GazL · 03-13-2023, 03:48 PM

int lines = file_num_lines(file);
char buf[lines + 1][256];

If you're going to dynamically size buffers then use malloc(). VLAs were added in C99, but they were downgraded to an "optional feature" in C11.

Unless you have a good reason not to (such as a recursive function where stack usage might matter it's often just easier to allocate a large enough buffer to hold the largest entry rather than try and do anything clever.

jmgibson1981 · 03-14-2023, 12:07 AM

Ok thank you. Will make some adjustments and report back. Thank you again.

pan64 · 03-14-2023, 01:11 PM

the best tool to catch that kind of errors is valgrind. (ok, probably not the best, but definitely a very good tool).

jmgibson1981 · 03-15-2023, 01:14 AM

Got it working. I followed the above suggestions. This now does both file or stdout depending on the flag used. I can see that I need to spend more time with validating functions.

Code:

void file_reverse_output(FILE * file, FILE * newfile, const char type)
{
  // declare and initialize count + buffers
  int lines = file_num_lines(file);
  char buf[lines + 1][256];
  char str[256];

  // load buffer with lines
  for (int i = lines - 1; i >= 0; i--) {
    char * ptr = fgets(str, 256, file);
    if (!ptr) {
      printf("buffer loading error!\n");
      printf("exiting. check line %d of your file.\n", i);
      exit(1);
    }
    strcpy(buf[i], ptr);
  }

  // output buffer in reverse order to file or screen
  // type is (p)rintf or (f)printf
  for (int i = 0; i <= lines - 1; i++) {
    if (type == 'p') {
      printf("%s", buf[i]);
    } else if (type == 'f') {
      fprintf(newfile, "%s", buf[i]);
    }
  }

  // reset file to beginning
  rewind(file);
}

NevemTeve · 03-15-2023, 01:54 AM

Also you can use `stdout` as `newfile` and drop parameter `type`.

Owel · 03-23-2023, 04:16 AM

I suggest you learning Valgrind and GDB. With these two tools your debugging process will be easier.