Segfault while reading last n bytes from a file

Question

I am trying to print last n bytes from a file. This is my code. I got segmentation fault. Please someone tell what is the error? Following is the code-

#include <stdio.h>
#include <ctype.h>
void fileprint(int line,char s[]);  //to print last n lines
int atof(char s[]);
int main(int argc, char * argv[])   // tail filename 10
{
    if(argc == 1)
        printf("usage: tail");
    if(argc == 2)
        fileprint(10, argv[1]);
    if(argc == 3)
        fileprint(atof(argv[2]), argv[1]);
    if(argc > 3)
        printf("usage: tail ");
}

void fileprint(int line, char s[])
{
    int c;
    FILE * p = fopen(s, "r");    // pointer to file
    fseek(p, -line, SEEK_END);
    while((c = fgetc(p)) != EOF) {
        putchar(c);
    }
}

int atof(char s[])     //convert string int
{
    int i,n;
    for(n = 0;isdigit(s[i]); i++)
        n = 10 * n + (s[i] - '0');
    return n;
}

Nope. Because you included line numbers and the formatting is horrible. You also didn't specify what the problem is other than dumping a bunch of code on us. — StoryTeller - Unslander Monica, Mar 19 '17 at 08:16
@KeineLust a negative offset is definitely allowed, and is the intended way to use fseek with SEEK_END — Horia Coman, Mar 19 '17 at 08:19
What arguments have you passed to main? Separately; you have redefined `atof()` (already in stdlib.h), but somewhat strangely you have changed its function also - a function not at all reflected in the name. Why would you do that? What is wrong with the standard library (and more appropriately named) `atoi()`? — Clifford, Mar 19 '17 at 08:41
You assign `p` and pass it to `fseek()` without checking that `fopen()` did not fail. If the file has failed to open (because it does not exist for example), `p` will be null, and dereferencing null is an error. — Clifford, Mar 19 '17 at 08:48
Not a cause of segfault, but you may get erroneous results from `fseek()` on some platforms (certainly Windows) due to line-end translation (two bytes CR+LF changed to single LF) if you do not use "rb" to open the file. http://stackoverflow.com/questions/780303/using-fseek-to-backtrack — Clifford, Mar 19 '17 at 08:56
the function: `atof()` is a well known function that converts a string to a `double`. It is a poor programming practice to use system function names for your application function names. — user3629249, Mar 19 '17 at 09:40
when calling `fopen()`, always check (!=NULL) the returned value to assure the operation was successful. When calling: `fseek()`, always check the returned value to assure the operation was successful — user3629249, Mar 19 '17 at 09:45
Error messages should be output to `stderr`, not `stdout`, The 'usage' statement is not correct: Suggest: `fprintf( "USAGE: %s [numTailChars(default:10)]\n", argv[0] );` Suggest using a `switch()` statement on `argc` to simplify the code. The switch() would have `case 2:` and `case 3:` and `default:` Where the `default` case would have the 'usage' statement. — user3629249, Mar 19 '17 at 09:54

score 0 · Answer 1 · answered Mar 19 '17 at 10:03

in your function: atof(),

the variable i is not initialized.

it contains what ever trash was on the stack at its' location.

This expression: isdigit(s[i]) is accessing some random place in memory.

This is undefined behavior and can/will lead to a seg fault event.

Segfault while reading last n bytes from a file

1 Answers1