Gavin Z.
Gavin Z.

Reputation: 431

Segmentation fault (core dump) in C?

This program basically search for the string in a file, and then produce the output based on the options provided.

For example, on the command line,

cmatch -i hello helloworld.txt

I keep getting the segmentation fault, even if I comment out all the printf statement. But I'm new to C and I can't find any inappropriate pointer here.. Help?

#include <errno.h>
#include <libgen.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <stdbool.h>

int exit_status = EXIT_SUCCESS;

bool checkIfMatch(char *string, FILE *file) {

    while (true) {
        char buff[1024];
        fgets(buff, 1024, file);
        if (buff == NULL)
            break;
        if (strcmp(string, strstr(buff, string)) == 0)
            return true;
    }
    return false;

}


bool checkIfMatchIgnoreCase(char *string, FILE *file){
    while (true) {
        char buff[1024];
        fgets(buff, 1024, file);
        if (buff == NULL)
            break;
        if (strcasecmp(string, strstr(buff, string))== 0)
            return true;
    }
    return false;

}

void printLines(char *string, FILE *file){
    while (true) {
        char buff[1024];
        fgets(buff, 1024, file);
            if (buff == NULL)
                break;
            if (strcmp(string, strstr(buff, string)) == 0)
                printf("%s",buff);
        }
}

void printLinesWithNumber(char *string, FILE *file){
    int ln;
    for(ln=1;;ln++) {
            char buff[1024];
            fgets(buff, 1024, file);
            if (buff == NULL)
                break;
            if (strcmp(string, strstr(buff, string)) == 0)
                printf("%d: %s",ln,buff);
        }
}

typedef struct options {
    bool ignore_case;
    bool filenames_only;
    bool number_lines;
} options;

void scan_options(int argc, char **argv, options *opts) {
    opts->ignore_case = false;
    opts->filenames_only = false;
    opts->number_lines = false;
    opterr = false;
    for (;;) {
        int opt = getopt(argc, argv, "iln");
        if (opt == EOF)
            break;
        switch (opt) {
        case 'i':
            opts->ignore_case = true;
            break;
        case 'l':
            opts->filenames_only = true;
            break;
        case 'n':
            opts->number_lines = true;
            break;
        default:
            exit_status = EXIT_FAILURE;
            fflush(NULL);
           fprintf(stderr, "%s: -%c: invalid option\n", 
                "cmatch.c", optopt);
            break;
        }
    }
}


int main(int argc, char **argv) {
    options opts;
    scan_options(argc, argv, &opts);
    char *stringToMatch = argv[optind];

    int i;
    for(i=1;i<=argc-optind-1;i++){

    bool matched;
    FILE *file=fopen(argv[optind+i],"r");
    if(opts.ignore_case) matched=checkIfMatch(stringToMatch, file);
    else matched=checkIfMatchIgnoreCase(stringToMatch, file);

    if(matched){
        if(opts.filenames_only) printf("%s\n",argv[optind+i]);
        if(opts.number_lines) printLinesWithNumber(stringToMatch, file);
        else printLines(stringToMatch,file);
    }
    }

    return exit_status;
}

Upvotes: 0

Views: 596

Answers (2)

user472308
user472308

Reputation:

You never check if fopen() actually worked.

FILE *file=fopen(...);
if (NULL != f) {    /* If the file was actually opened */
    /* Do the work */
}

Also, do you really have a file named argv[optind+i] ? Maybe you meant to omit the quotes:

fopen(argv[optind+i],"r");

Also, it looks like you need to use fseek() or rewind() before the successive calls to fgets() http://www.cplusplus.com/reference/cstdio/fseek/.

Finally, reading from file is slow. You really should just read the file once in one place and then reuse what is then in memory.

Ok, you've fixed the file problems. Now using gdb, I found out that your first segmentation fault happens on line 18 in strcmp() because strstr() returns NULL if the sequence is not found in buff:

    if (strcmp(string, strstr(buff, string)) == 0)
        return true;

You need to move strstr() out of the parameters to strcmp() and check for NULL before passing the result to strcmp(). http://www.cplusplus.com/reference/cstring/strstr/

Upvotes: 0

cnicutar
cnicutar

Reputation: 182619

char *buff = fgets(buff, 1024, file);
      ^^^          ^^^

You need to call fgets with memory that you have already allocated. Something like:

char buff[1024];
fgets(buff, sizeof buff, file);

Upvotes: 7

Related Questions