0

my code keeps throwing a segmentation fault from internal c libraries, my code is the following:

        char *vertexShaderCode = (char *)calloc(1024, sizeof(char));
        FILE *shaderFile;

        shaderFile = fopen("./shaders/vertex.glsl", "r");

        if(shaderFile)
        {
            //TODO: load file
            for (char *line; !feof(shaderFile);)
            {
                fgets(line, 1024, shaderFile);
                strcat(vertexShaderCode, line);
            }

it is meant to load all the data from a file as a c string, line by line. can anyone help?

  • [`while (!feof(file))`](https://stackoverflow.com/questions/5431941/), even when disguised, [is always wrong](https://stackoverflow.com/questions/5431941/) – pmg May 24 '20 at 15:57
  • 1
    TODO: check if `fopen` fails. – Jabberwocky May 24 '20 at 15:59
  • _" keeps throwing errors"_: which errors? Post the __verbatim__ error messages in your question. You can [Edit] your question. – Jabberwocky May 24 '20 at 16:00
  • i keep getting a segmentation fault from my program, i edited that into the question – Shadow Lynch May 24 '20 at 16:04
  • 1
    1. do what I told you in my second comment. 2. eliminate the wrong `feof` as per the first comment. 3. make sure you allocate enough memory with `calloc`, if your file is too long, you'll have a buffer overflow which often manifests itself as segmentation fault. – Jabberwocky May 24 '20 at 16:07
  • i checked for the fopen error, that is not the issue, and i fixed the segmentation fault. I have no idea how though... my code is now in the question – Shadow Lynch May 24 '20 at 16:10
  • 2
    I concur with @Jabberwocky, most likely your file is more than 1024 characters and you aren't calling `realloc` on your buffer – iggy12345 May 24 '20 at 16:10

1 Answers1

1

You want this:

char *vertexShaderCode = (char *)calloc(1024, sizeof(char));
FILE *shaderFile;

shaderFile = fopen("./shaders/vertex.glsl", "r");
if (shaderFile == NULL)
{
   printf("Could not open file, bye.");
   exit(1);
}

char line[1024];
while (fgets(line, sizeof(line), shaderFile) != NULL)
{
   strcat(vertexShaderCode, line);
}

You still need to make your that there is no buffer overflow. Possibly you need touse realloc in order to expand the buffer if the initial length of the buffer is too small. I leave this as an exercise to you.


Your wrong code:

    char *vertexShaderCode = (char *)calloc(1024, sizeof(char));
    FILE *shaderFile;

    shaderFile = fopen("./shaders/vertex.glsl", "r");  // no check if fopen fails

    for (char *line; !feof(shaderFile);)   // wrong usage of feof
    {                                      // line is not initialized
                                           // that's the main problem
        fgets(line, 1024, shaderFile);
        strcat(vertexShaderCode, line);    // no check if buffer overflows
    }
Jabberwocky
  • 48,281
  • 17
  • 65
  • 115
  • how would i account for the buffer overflow, i don't know how to do that. – Shadow Lynch May 24 '20 at 16:15
  • @ShadowLynch your code is basically accumulating information in the `vertexShaderCode` buffer whose size if 1024. If you continue to add information in that buffer it will overflow, just like if you continue to pour water in a glass when it it full, the water will overflow. Hint: use a pencil and a piece of paper. Hint 2: use the `realloc` function to expand your buffer if it gets too small. – Jabberwocky May 24 '20 at 16:19
  • In the while loop you could check that `strlen(line) + strlen(vertexShaderCode)` is less than 1024. If not it could be a good place where to add realloc (or at least a warning printf). – Roberto Caboni May 24 '20 at 16:26