Why is this code segfaulting? (Pointer arithmetic)

Question

For some reason this code is segfaulting for a reason I can't find out.

char *read_line(FILE *fp)
{
    char *out;
    int counter = 0;
    char c = getc(fp);

    while (c != '\n' && c != EOF)
    {
        *(out + counter) = c;
        counter++;
        c = getc(fp);
    }

    if (c == EOF || feof(fp))
    {
        return NULL;
    }

    *(out + counter) = '\0';
    return out;
}

I have already tried running it in gdb, and that has told me that the segfault is at *(out + counter) = c;. I can't figure out what I am doing wrong, can anyone else?

If you want to read data to somewhere you need to provide the memory to do so. As the code currently stands `out` is just apointer to nowhere. And writing to where it points to provokes undefined behaviour and a crash. — alk, Feb 09 '14 at 10:44
I understand how pointers work, but I don't see why this doesn't work. What confuses me is how this usually works, but sometimes it decides to segfault for no good reason. — AppleDash, Feb 09 '14 at 10:45

barak manos · Accepted Answer · 2014-02-09T10:52:06.247

6

You're not assigning any value to out, so it's probably pointing to some invalid memory address.

What you probably want to do is find the length of the data first, then allocating the required amount of memory, and then read the actual data into the allocated memory:

char *out;
int counter = 0;
char c = getc(fp);
while (c != '\n' && c != EOF) {
    counter++;
    c = getc(fp);
}

out = malloc(counter+1);
fseek(fp,0,SEEK_SET);

counter = 0;
c = getc(fp);
while (c != '\n' && c != EOF) {
    *(out + counter) = c;
    counter++;
    c = getc(fp);
}
*(out + counter) = 0;

And don't forget to free(out) when you're done using it...

BTW, instead of the second for loop, you could simply use fgets(out,count,fp).

edited Feb 09 '14 at 10:52

answered Feb 09 '14 at 10:40

barak manos

29,648
10
62
114

But what value WOULD I assign to it? – AppleDash Feb 09 '14 at 10:44
2

The address of an allocated piece of memory; see updated answer... – barak manos Feb 09 '14 at 10:45
@alk You could try that as well, but no guarantees :) – barak manos Feb 09 '14 at 10:49
1

`*(out + counter) = c;` - please don't. Readability is golden. – Feb 09 '14 at 10:50
@H2CO3 I can read that just fine. – AppleDash Feb 09 '14 at 10:51
@AppleDash doesn't mean you should do it like this. perhaps be respectful to others to read your code? that pointer arithmetic plus dereferencing thingy is unidiomatic, at best. – Feb 09 '14 at 10:53
@H2CO3, I agree, but I did not want to confuse OP any further, so copy-pasted from OP's question... – barak manos Feb 09 '14 at 10:53
@H2CO3 This code is mostly for a "troll" fork of a project anyways, so perhaps less readable code is better. I'm only using C because that's what the original project uses. – AppleDash Feb 09 '14 at 10:54

Why is this code segfaulting? (Pointer arithmetic)

1 Answers1