I'm getting a segfault trying to open a file and write it to another

Question

I know the segfault is located within the argv[y] but I don't understand why! I'm very new to UNIX and rusty on my C. Any help would be awesome. (printfs were a messy way to find where the segfault was!)

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

#define PAGELEN 24
#define LINELEN 512

void do_more(FILE *);
int see_more();

int main(int argc, char *argv[]) 
{
    FILE *fp;
    int i;
    int loc;
    int x;
    int y;
    char buffer[LINELEN];
    FILE *t;
    FILE *out;
    FILE *read;

    for(i=1; i<argc; i++){
        if(strcmp(argv[i], "-o") == 0)
            loc = i;
        else if((t = fopen(argv[i], "r")) != NULL){
            x = i;
            fclose(t);
        }
        else{
            y = i;
        }
    }

    if(loc != -1){
        FILE *read;

        printf("1");
        read = fopen(argv[x], "r");
        printf("2");
        out = fopen(argv[y], "w");
        printf("3");
        while(fgets(buffer, LINELEN, read))
            printf("4");
        fputs(buffer, out);
        printf("5");
        fclose(read);
        fclose(out);
    }
}

Also, consider using something like [getopt](https://www.gnu.org/software/libc/manual/html_node/Example-of-Getopt.html#Example-of-Getopt). — Fiddling Bits, Jan 24 '20 at 21:50
What exactly did you input and what are the values of `x` and `y` when the seg fault occurs? — kaylum, Jan 24 '20 at 21:50
This probably isn't causing your problem but you have two variables named `read`. The one in the `if(loc != -1)` block is shadowing the one in the `int main(int argc, char *argv[])` block. — Fiddling Bits, Jan 24 '20 at 21:56
It's not immediately obviously how to execute your program. Can you show an example? `main.exe in.txt -o out.txt` for example. — Fiddling Bits, Jan 24 '20 at 21:58
[`read`](http://man7.org/linux/man-pages/man2/read.2.html) is also the name of a function .. recommend not using that as a variable name to avoid confusion. — yano, Jan 24 '20 at 21:59
haha whoops, I was getting desperate so i was trying anything and everything — Daniel Ortman, Jan 24 '20 at 22:00
I'm curious what the values of `x`, `y`, and `loc` are after your `for` loop. These aren't initialized to anything (when will `loc` ever be -1?). If `x` or `y` is missing assignment in the `for` loop, they will be some garbage values and `argv[x or y]` will probably segfault — yano, Jan 24 '20 at 22:08
I don't know how to do break points within vim, or even if you can. This is were a lot of issues arise. If I were in VS I could put break points and tell you what each value is. — Daniel Ortman, Jan 24 '20 at 22:11
One of the more popular debuggers in linux is gdb ,,, highly recommend learning how to use it (or another) if you're going to do any kind of sustained development in linux. In the mean time, you can simply add `printf`s for those 3 variables after the `for` loop. Don't forget `printf` is [line buffered](https://stackoverflow.com/questions/1716296/why-does-printf-not-flush-after-the-call-unless-a-newline-is-in-the-format-strin), so add newlines or flush the output, or you may not see the printout before the crash. — yano, Jan 24 '20 at 22:23
Update: The segfault is tied into the loc = i; unsure how to fix now! I have updated loc to = -1; — Daniel Ortman, Jan 24 '20 at 22:29

score 1 · Answer 1 · answered Jan 24 '20 at 22:47

As yano mentioned, some variables can be left uninitialized, which will cause the segmentation fault. Enable compiler warnings (-Wall for most compilers) and fix all warnings it gives.

Using getopt() as Fiddling Bits mentioned would be great.

Another issue with your code is the check for whether an argument is the input file or the output file is bad. If the input file doesn't exist, or there is already a file with the same name as the output file, the check gives the wrong answer. Here is a better approach to parsing arguments, if you couldn't use getopt():

const char *input_filename = NULL;
const char *output_filename = NULL;

for (int i = 1; i < argc; i++) {
    if (strcmp(argv[i], "-o") == 0) {
        if (output_filename != NULL || i + 1 >= argc) {
            fprintf(stderr, "Error parsing arguments: double use of -o or missing filename\n");
            return 1;
        }

        output_filename = argv[++i];
    } else {
        if (input_filename != NULL) {
            fprintf(stderr, "Error parsing arguments: more than one input specified\n");
            return 1;
        }

        input_filename = argv[i];
    }
}

if (input_filename == NULL || output_filename == NULL) {
    fprintf(stderr, "Error parsing arguments: missing input and/or output filename\n");
    return 1;
}

FILE *read = fopen(input_filename, "r");
// add error checking here as well
FILE *out = fopen(output_filename, "w");
// add error checking here as well

...

anastaciu · Accepted Answer · 2020-01-24T22:41:14.263

So you had a couple of problems x and y are not being assigned correctly, if you harcode argv[1] instead of argv[x] and argv[3] instead of argv[y] it works fine, strangely enough the segfault only occours if the file to write already exists.

so try: code sample

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

#define PAGELEN 24
#define LINELEN 512

void do_more(FILE *); 
int see_more();

int main(int argc, char *argv[])   //   ./main file.txt -o out.txt
{
    FILE *fp;
    int i;
    int loc;
    int x;
    int y;
    char buffer[LINELEN];
    FILE *t;
    FILE *out;
    FILE *f; // <-- changed to avoid confusion

    for(i=1; i<argc; i++){
        if(strcmp(argv[i], "-o") == 0)
            loc = i;
        else if((t = fopen(argv[i], "r")) != NULL){
            x = i;
            fclose(t);
        }
        else{
            y = i;
        }
    }

    if(loc != -1){
        printf("1");
        f = fopen(argv[1], "r");// <-- argument 2
        printf("2");
        out = fopen(argv[3], "w");// <-- argument 4
        printf("3");
        while(fgets(buffer, LINELEN, f)){
            printf("4");
            fputs(buffer, out);
        }
        printf("5");
        fclose(f);
        fclose(out);
    }
}

Since your arguments don't seem to change you can leave it like that or, if you'd like, correct the argument selection for opening.

The getopt mentioned in the comments is always a good option for this.

You still have to account for the errors when the argument input is incorrect, this will still cause errors, but I will leave that for you.

Thank you very much! I believe it was tied up in the *read/ what you renamed to f. After changing that it works perfectly with the x and y variables as well as hard coding in 1->x and 3->y. — Daniel Ortman, Jan 24 '20 at 22:45

I'm getting a segfault trying to open a file and write it to another

2 Answers2