Titas B
Titas B

Reputation: 46

Deleting a redundant parameter results in a segmentation fault, seemingly for no reason

The following program finds and deletes words that begin and end with the same character. It works just fine, except I decided to take the code for printing result text in from deleteWords() and put it inside of main(). Therefore, the *fpOut parameter in became redundant in deleteWords(). Deleting the parameter results in

/bin/sh: line 1: 1371 Segmentation fault: 11 ./main duom.txt rez.txt make: *** [main] Error 139

However if I compile it and run it any third parameter (e.g. int useless argument instead of FILE *fpOut), it works without errors.

Has anybody have a clue what could be causing this problem?

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

    int checker (char zodis[]) {
        size_t last = strlen(zodis);
        if (zodis[0] == zodis[last-1])
            return 0;
        return 1;
    }

    void memAlloc (char **text, char **buffer, FILE **fp, char *fileName) {
        int fileLength;
        *fp = fopen(fileName, "r");
        fseek(*fp, 0L, SEEK_END);
        fileLength = fseek(*fp, 0L, SEEK_SET);
        *text = malloc(fileLength * sizeof(char));
        *buffer = malloc(fileLength * sizeof(char));
    }

    void deleteWords (FILE *fp, int anyUselessParameter, char *buffer) {
        char *text;
        while (fscanf(fp, "%s", text) == 1) {
            if (checker(text)) {
                printf("%s ", text);
                sprintf(buffer + strlen(buffer), "%s ", text);
            }
        }

        printf("\n");
    }

    int main(int argc, char *argv[]) {
        FILE *fp, *fpOut;
        int anyUselessParameter;
        char *text, *buffer, *inputFileName = argv[1], *outputFileName = argv[2];

        if (argc < 2)
            return 0;

        fpOut = fopen(outputFileName, "w");

        memAlloc(&text, &buffer, &fp, inputFileName);
        deleteWords(fp, anyUselessParameter, buffer);
        fputs(buffer, fpOut);

        fclose(fp);
        fclose(fpOut);
        free(text);
        return 0;
    }

Upvotes: 0

Views: 63

Answers (2)

KamilCuk
KamilCuk

Reputation: 141623

char *text;
while (fscanf(fp, "%s", text) == 1) {

scanf needs the buffer to be allocated. Here it dereferences an uninitialized pointer text and writes to it. scanf tries to write to text[0], text[1].. and so on, so accesses text out of bounds and undefined behavior happen.

*buffer = malloc(fileLength * sizeof(char));
...
sprintf(buffer + strlen(buffer), "%s ", text);

buffer is uninitialized, so strlen(buffer) will result in some undefined value. Explicitly initialize buffer[0] = '\0' if you wish to use strlen later. Also you don't include memory for terminating '\0' character inside your buffer.

As you are trying to read the file into a buffer, that is allocated using the file size

if (fread(buffer, fileLenght, 1, fp) != fileLength) { /* handle error */ }

If you have to, use snprintf instead of sprintf just to be safe. snprinttf(buffer+strlen(buffer), fileLength - strlen(buffer), ...);

Also, try to never use scanf without specifing field length inside %s modifier. You can try:

char text[256]; // or other maximum word length
while (fscanf(fp, "%255s", text) == 1) {

As you already have allocated memory for the file, you can use it as a parameter to scanf, if you have to. One would need to prepare the format string for scanf as argument - it is a bit hard. See below:

 for (;;) {
      // prepare scanf %s format modifier to use with printf to write to buffer end
      char fmt[20];
      size_t buffer_size = fileLenght;
      size_t free_in_buffer = buffer_size - strlen(buffer);
      snprintf(fmt, 20, "%%%ds", free_in_buffer);
      // we will write here: up to free_in_buffer
      char *in = buffer + strlen(buffer);
      if (fscanf(fp, fmt, in) != 1) break;
      // we now check the last readed word form the file
      if (!checker(in)) { 
         // if the last readed word is bad, we can revert it
         in[0] = '\0'
      }
  }

Upvotes: 4

Andrew Henle
Andrew Henle

Reputation: 1

This is wrong:

fileLength = fseek(*fp, 0L, SEEK_SET);

Per POSIX:

RETURN VALUE

The fseek() and fseeko() functions shall return 0 if they succeed.

Otherwise, they shall return -1 and set errno to indicate the error.

Upvotes: 3

Related Questions