strtok segmentation fault related to string termination

Question

I am writing a program that read a text file line by line and delete a specific word in each line, then print new lines to another text file.

I keep getting segmentation fault in Zeus, but my code runs perfectly in Visual Studio. I am required to run this program on Zeus so I have to figure out what is the problem. I figured out that one line of my code caused the problem. Please check it below.

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

int main()
{
    FILE *myfile;
    FILE *des;              //store edited text file
    char buf[101];          //store line by line
    int pos;                //the position of the deleted word
    char deleted[100] = ""; //store deleted words
    char input[100] = "";   //take input file name
    char output[100] = "";  //take output file name

    printf("Enter the name of the input file: ");
//  scanf(" %s", &input);
    printf("Enter the name of the output file: ");
    //scanf(" %s", &output);

    myfile = fopen("Lab6_bad.txt", "r"); //read file
    des = fopen("Lab6_good.txt", "w");   //open a file for writing

                                //check if the text file exists
    if (myfile == NULL)
    {
        printf("The file does not exist.\n");
        exit(0);
    }

    //read text file line by line using fgets()
    while (fgets(buf, sizeof(buf), myfile) != NULL)
    {
        char newline[101] = ""; //store the new line

        printf("%s", buf);

buf[sizeof(buf)] = '\0'; //terminate

                                 //prevent reading the line twice
        if (buf[0] == '\n')
        {
            break;
        }

        printf("%s", buf);
        printf("Enter position of word to delete (Start counting at 0). Enter -1 to skip deletion: ");
        scanf(" %d", &pos);
        printf("\n");

        //not edit a line
        if (pos == -1)
        {
            fprintf(des, "%s\n", buf);
            continue;
        }

        char *token;
        int count = 0; //record the current token's position

token = strtok(buf, " "); //get first token. Problen happens here !!! Looks like Zeus does not add '\0' automatically, so i add one line above( bold line), but it still doesn't work

        while (token != NULL)
        {
            if (count == pos)
            {
                strcat(deleted, token);
                strcat(deleted, " ");
            }
            else
            {
                strcat(newline, token);
                strcat(newline, " ");
            }
            token = strtok(NULL, " ");
            count++;
        }
        fprintf(des, "%s\n", newline);
    }

    fclose(myfile);
    fclose(des);
    printf("%s", deleted);
}

What is "Zeus"? If it is an IDE with a debugger then suggest you use the debugger to help you find the problem. — kaylum
buf[sizeof(buf)] = '\0'; that is a buffer overflow and thus results in Undefined Behaviour. fgets already guarantees a terminating NUL byte so you don't have to manually NUL terminate. But if you really must then it needs to be buf[(sizeof buf)-1] = '\0'; — kaylum
this doesn't work. I actually run with the code i posted above without any problem once. But after I added some comments, it can't run. I don't know why and really got confused — user4593157
You have Undefined Behaviour. The code is wrong. But due to UB it may sometimes appear to work. You MUST fix that bug or it is a ticking time bomb and I can guarantee you that it will stop working at some point even in VS. It doesn't mean that fixing this one bug will make your program work completely. But it has to be fixed regardless of anything else. There is no point debugging further if your code clearly contains UB that has not been fixed. — kaylum

chqrlie chqrlie · Accepted Answer · 2016-03-13T23:55:52

fgets() does null terminate the buffer into which it stores the line read.

Your statement: buf[sizeof(buf)] = '\0'; actually invokes undefined behavior! you store a NUL character beyond the end of the array.

Conversely, you do not test the return value of scanf(" %d", &pos);. If standard input does not parse as a number, 0 or possibly EOF is returned and pos is left uninitialized, causing incorrect behavior.

More important: you concatenate all deleted words into that array deleted but never check for buffer overflow. If you delete too many words, the buffer will eventually be too small and undefined behavior will be invoked.

strtok segmentation fault related to string termination

1 Answers