LinuxQuestions.org

LinuxQuestions.org (/questions/)
-   Programming (https://www.linuxquestions.org/questions/programming-9/)
-   -   Getting a segfault. Can't find it. (https://www.linuxquestions.org/questions/programming-9/getting-a-segfault-cant-find-it-4175722983/)

jmgibson1981 03-13-2023 12:22 AM
Getting a segfault. Can't find it.
 
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.


All times are GMT -5. The time now is 12:36 PM.