Segmentation fault on a read function

Question

I have got some serious problems with Stack Smash Protection and now I get a new error -Segmentation fault-. I think it is strongly related to the fact that linux has some special protections. Can anyone please explain me why do I get Segmentation fault on this particular case?

vector<const char*> Words;
void read(){
    FILE* a;
    char s[100];
    a = fopen("text.txt", "r");
    while (!feof(a))
    {
        fgets(s, 100, a);
        char *newAlloc = new char[strlen(s)];
        strcpy(newAlloc, s);
        Words.push_back(newAlloc);
    }
    fclose(a);
}

update: I tried all of the solutions and modified the code, but the problem is still there, so I tried to reduce the code to this:

#include<iostream>
#include<stdio.h>

int main()
{

 FILE* a;
 a=fopen("text.txt", "r");
 fclose(a);

 }

It still gives me that error at the line with fopen.(which is mandatory in the exercise I'm solving) - I'm using Ubuntu 15.10 and QT Creator along with GCC compiler.

update: Solved it. I guess the problem was because I didn't give the full path to fopen. I'm new with ubuntu. Apparently there are some things different.

 char * a = "/home/codrinz/text.txt";
 FILE * file = fopen(a, "r");

What an unholy mix of C and C++. Get rid of C-style strings, replace them with `std::string`, and you will get rid of your problems. Do the same for file-based IO as well (right now you do not check returned pointer, which might be nullptr). — SergeyA, Apr 05 '16 at 20:13

score 3 · Accepted Answer · edited May 23 '17 at 11:45

3

I see couple of problems.

Don't use while (!foeof(a)). See Why is “while ( !feof (file) )” always wrong?.
You are not allocating sufficient memory for the words. As a consequence, you end up using memory that you are not supposed to. This leads to undefined behavior.

Use:

while (fgets(s, 100, a))
{
   char *newAlloc = new char[strlen(s) + 1];  // Add +1 for the terminating null character.
   strcpy(newAlloc, s);
   Words.push_back(newAlloc);
}

edited May 23 '17 at 11:45

Community

1
1

answered Apr 05 '16 at 20:15

R Sahu

204,454
14
159
270

1

I don't understand why `strdup()` isn't more widely used instead of the `char* t = malloc(strlen(s) + 1);strcpy(t, s);` pattern (or maybe anti-pattern). – Michael Burr Apr 05 '16 at 20:21
@MichaelBurr Most likely because it is not a standard library function. – R Sahu Apr 05 '16 at 20:23
You are probably right. But it's so easy to write it (or an equivalent) once and avoid bugs like this forever. Based on SO questions, this is something that bites people a lot. I think even non-beginners probably get bit more than they'd like. – Michael Burr Apr 05 '16 at 20:31
@MichaelBurr, True, there are A LOT of questions on SO with that error. I don't program C on a daily basis. I don't know how many experienced programmers get bit by this. – R Sahu Apr 05 '16 at 20:36
@MichaelBurr in C++ you should not allocate memory that way in the first place – M.M Apr 05 '16 at 21:54

Segmentation fault on a read function

1 Answers1