Segmentation Fault in Program that Reads the Last 10 lines From Command Line

Question

I am currently trying to create a program that prints the last 10 lines of another text program that is passed in through the command line. The function read() should read a single line from the text file and then return either the string or NULL, while the main function should keep assigning the output from read() until NULL is assigned.

I have gone through many different versions of this program, but it always ends up causing a segmentation fault. How would I begin to debug the segmentation fault?

I have included the program below.

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

char *read(){
   char *line = (char *) malloc (80 * sizeof(char));
   fgets(line,80,stdin);
   if (line != NULL) {
      line[strlen(line)-1] = '\0';
      return line;
   }
   else {
      free(line);
      return NULL;
   }
}

int main()
{
   int i=0,j,k,l;
   char **arr = (char **) malloc (100 * sizeof(char *));
   for (k = 0; k < 100; k++) {
      arr[k] = malloc (80 * sizeof(char));
   }

   while(1){
      strcpy(arr[i],read());
      if (arr[i]=NULL) break;
      i++;
      //printf("%s", arr[i]);  //DEBUG
   }

   for (l = 0; l < 100; l++){
      free(arr[l]);
   }
   free(arr);

   for (j = i-11; j < i; j++) {
      printf("%s\n", arr[j]);
   }
   printf("\n");

   return 0;
}

Use a debugger to identify WHERE the set fault occurs, and work from there. — Scott Hunter, Feb 23 '19 at 19:11
Assuming the function works properly you must check its return value for `NULL` *before* passing it to `strcpy`. Also `line[strlen(line)-1] = '\0';` will cause havoc if there is no newline (perhaps the last line in the file), see [Removing trailing newline character from fgets() input](https://stackoverflow.com/questions/2693776/removing-trailing-newline-character-from-fgets-input/28462221#28462221). You also have a memory leak in your `read` function. Lastly it is a bad idea to name it `read` because there is a standard library function by that name, which does something different. — Weather Vane, Feb 23 '19 at 19:22

bruno · Accepted Answer · 2019-02-23T19:54:22.947

in main

while(1){
  strcpy(arr[i],read());
  if (arr[i]=NULL) break;

you never go out of the loop because arr[i]=NULL has no reason to be true from read (because of an error in read), so you write out of arr with an undefined behavior (your crash)

You do not well manage the end of file in read :

fgets(line,80,stdin);
if (line != NULL) {

you need to check the result of fgets not if line is NULL except to check malloc success

not that line[strlen(line)-1] = '\0'; is useless because fgets put a final null character, fortunately else how strlen can work ?

So read can be :

char *read(){
  char *line = (char *) malloc (80 * sizeof(char));

   if (line != NULL) {
     if (fgets(line,80,stdin) == NULL) {
       free(line);
       return NULL;
     }
   }

   return line;
}

Because read can return a NULL you need to change

  strcpy(arr[i],read());

and also is why you preallocate the lines while you also allocate in read ? currently you loose the memory allocated each time in read

It seems better to replace the loop by

while((arr[i] == read()) != NULL)
   //printf("%s", arr[i]);  //DEBUG
   i += 1;
}

and to remove the preallocations of the lines

An other problem is when the file have more that 100 lines (or may be some lines are cut before too long), in that case you write out of arr. In fact you do not need to save so many lines, you just need to save 10 lines maximum

A proposal :

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

char *read() {
  char line[80];

  if (fgets(line, 80, stdin) == NULL)
      return NULL;

   return strdup(line);
}

int main()
{
  char * arr[10] = { NULL };
  char * p;
  int i = 0, n = 0;

  while((p = read()) != NULL) {
    n += 1;
    free(arr[i]);
    arr[i] = p;
    if (++i == 10)
      i = 0;
  }

  /* get older line index and line count to print */
  if (n <= 10) 
    i = 0;
  else
    n = 10;

  while (n-- != 0) {
    puts(arr[i]);
    free(arr[i]);
    if (++i == 10)
      i = 0;
  }

   return 0;
}

Compilation and execution :
pi@raspberrypi:/tmp/d $ gcc -g -pedantic -Wextra q.c
pi@raspberrypi:/tmp/d $ ./a.out < q.c
    n = 10;



  while (n-- != 0) {

    puts(arr[i]);

    if (++i == 10)

      i = 0;

  }



   return 0;

}

Note I do not removed the \n is the read lines and I use puts so there are empty lines printed while the read line are not cut because too long

If the file is shorter that 10 lines :

pi@raspberrypi:/tmp/d $ tail -n 5 q.c | ./a.out
      i = 0;

  }



   return 0;

}

Under valgrind :

pi@raspberrypi:/tmp/d $ valgrind ./a.out < q.c
==18496== Memcheck, a memory error detector
==18496== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==18496== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==18496== Command: ./a.out
==18496== 


  while (n-- != 0) {

    puts(arr[i]);

    free(arr[i]);

    if (++i == 10)

      i = 0;

  }



   return 0;

}

==18496== 
==18496== HEAP SUMMARY:
==18496==     in use at exit: 0 bytes in 0 blocks
==18496==   total heap usage: 43 allocs, 43 frees, 5,699 bytes allocated
==18496== 
==18496== All heap blocks were freed -- no leaks are possible
==18496== 
==18496== For counts of detected and suppressed errors, rerun with: -v
==18496== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 6 from 3)

Segmentation Fault in Program that Reads the Last 10 lines From Command Line

1 Answers1