1

My project is trying to load a font for rendering from a zip file. I use LibZip for accessing the zip, and the procedure I use works fine for all the graphics I'm rendering. However, using the same process for loading a ttf font eventually causes a segfault when I try to render the text using SDL_ttf's TTF_RenderText functions. I've tested that that's the cause by replacing the loading code with TTF_Open and successfully gotten it to load, while loading from zip using my code ends with segfault. Here's the code I use to load the file from the zip:

TTF_Font* m_load_font(const char* filename, int fontsize) {
  zip_stat_t filestat;
  uint64_t filetotal;
  SDL_RWops* rwop = SDL_AllocRW();

  //Check to see if the data stream got allocated
  if ( rwop == NULL ) {
    u_error(std::cerr, CC_ERROR_SDL, "SDL_AllocRW");
    SDL_FreeRW(rwop);
    return nullptr;
  }
  //Check to see if the resource zip is open
  if ( cc_zip_res == NULL ) {
    std::cerr << "Font Load Error: Zip File Not Loaded" << std::endl;
    SDL_FreeRW(rwop);
    return nullptr;
  }
  //Open the file
  zip_file_t* file = zip_fopen(cc_zip_res, filename, 0);
  if (file == NULL) {
    u_error(std::cerr, CC_ERROR_ZIP, "zip_fopen");
    zip_fclose(file);
    SDL_FreeRW(rwop);
    return nullptr;
  }
  //Read the file
  if(zip_stat(cc_zip_res, filename, 0, &filestat) == -1) {
    u_error(std::cerr, CC_ERROR_ZIP, "zip_stat");
    zip_fclose(file);
    SDL_FreeRW(rwop);
    return nullptr;
  }
  //Write data to a data holding array
  char* rwbuffer = new char[filestat.size];
  rwop = SDL_RWFromMem(rwbuffer, filestat.size);
  while(filetotal < filestat.size) {
    char buffer[256];
    int64_t length;
    //read the file into the buffer
    length = zip_fread(file, buffer, 256);
    if (length == -1) {
      u_error(std::cerr, CC_ERROR_ZIP, "zip_fread");
      zip_fclose(file);
      SDL_FreeRW(rwop);
      delete[] rwbuffer;
      return nullptr;
    }
    //write the buffer into the rwop stream
    if ( (size_t)length != SDL_RWwrite(rwop, buffer, 1, (size_t)length) ) {
      u_error(std::cerr, CC_ERROR_SDL, "SDL_RWwrite");
      zip_fclose(file);
      SDL_FreeRW(rwop);
      delete[] rwbuffer;
      return nullptr;
    }
    //Increment the count so that the loop ends
    filetotal += length;
  }
  zip_fclose(file);
  //Return the seek pointer of the RWop to the beginning so that the file can be read
  SDL_RWseek(rwop,0,RW_SEEK_SET);
  //Load the font from the rwop
  TTF_Font* temp = TTF_OpenFontRW(rwop, 0, fontsize);
  SDL_FreeRW(rwop);
  delete[] rwbuffer;
  if (temp == NULL) {
    u_error(std::cerr, CC_ERROR_TTF, "TTF_OpenFontRw");
  }
  return temp;
}

And here's the code I'm using that has TTF_RenderText:

bool CC_Texture::load_from_font(TTF_Font* font, std::string* text, SDL_Color color,
  SDL_Renderer* ren) {
  if (font == NULL) {
    std::cerr << "Font is Null in CC_Texture::load_from_font" << std::endl;
    return false;
  }
  //The wrap length is an estimate, will figure out the exact
  SDL_Surface* surf = TTF_RenderText_Blended_Wrapped(font, text->c_str(), color, 600);
  if (surf == NULL) {
    u_error(std::cerr, CC_ERROR_TTF, "TTF_RenderText_Blended_Wrapped");
    return false;
  }
  texture = std::shared_ptr<SDL_Texture>(SDL_CreateTextureFromSurface(ren, surf),
    SDL_DestroyTexture);
  if (texture.get() == NULL) {
    u_error(std::cerr, CC_ERROR_SDL, "SDL_CreateTextureFromSurface");
    return false;
  }
  return true;
}

EDIT: I should also say that I used a debugger to dump the contents of rwbuffer after it did the load and it was a valid font file, so it doesn't seem like the zip reading is the problem.

genpfault
  • 51,148
  • 11
  • 85
  • 139
laura
  • 51
  • 5
  • Not the answer, but you might want to read about *scope guards*. Then you won't need to duplicate resource-freeing code ever again. – HolyBlackCat May 26 '20 at 23:23
  • 2
    I'm not entirely sure, but I think the RWops must stay valid while the font is used. – HolyBlackCat May 26 '20 at 23:30
  • That, plus leaking rwops with pointless `AllocRW` and never freeing it. – keltar May 27 '20 at 02:20
  • @whitehawk It's kinda strange to try to solve your problem by putting up a bounty here. You should rather post a separate question with a [mcve] for your code. – HolyBlackCat Jul 28 '23 at 17:13
  • [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) – Jesper Juhl Jul 30 '23 at 20:44

1 Answers1

1

TD;DR

The problem

You freed the SDL_RWops and its buffer right after opening them using TTF_OpenFontRW, even though such a TTF_Font is closely linked to those and freeing them essentially means breaking it, as it'll make the TTF_Font try to access memory that you freed, resulting in a segfault error.

The solution

Remove the SDL_FreeRW(rwop);and delete[] rwbuffer; line from your m_load_fontfunction, and put them wherever you close your font.

To simplify things, you can make the second parameter of TTF_OpenFontRW a 1 instead of 0 to have rwop automatically closed whenever you close your font, but you will still need to delete[] rwbuffer; manually, right after you close your font.

Note for the bounty comment

I'm keeping my RWops alive and not freeing it.

Make sure you keep BOTH your RWops AND the buffer that is linked to it alive. The RWops alone isn't enough, both the RWops and the buffer you have opened it with must NOT be freed for as long as you are using your font.

The long answer

How does SDL_RWOps work

What the documentation says

According the documentation for the SDL_RWFromMem function (mirror link):

This memory buffer is not copied by the SDL_RWops; the pointer you provide must remain valid until you close the stream. Closing the stream will not free the original buffer.

What does this mean for us?

What this means is that you can free the buffer linked to your SDL_RWops only when you are done using said buffer. Otherwise, it'd make your SDL_RWops instance invalid, because it will try to read into memory that has been read, resulting in a segmentation fault.

How does TTF_OpenFontRW work

What the documentation implies

For this function, the documentation (mirror link) says the following:

If freesrc is non-zero, the SDL_RWops will be closed before returning, whether this function succeeds or not. SDL_ttf reads everything it needs from the SDL_RWops during this call in any case.

We don't really care about the freesrc part of the sentence here, what we care about is the other part, which says that in any case (implying no matter the parameters), the TTF_OpenFontRW function will read everything it needs from the SDL_RWops, which implies that the SDL_RWops is no longer needed after calling this function, right?

It turns out that this statement is wrong. See below.

TTF_OpenFontRW's actual implementation

At the time of writing, here is how the TTF_OpenFontRW function is implemented:

As can be seen on line 1800, the pointer to the SDL_RWops passed to the function is stored in the TTF_Font instance that'll be returned to you.

On line 1812, we can also see that it is stored in the structure of a stream that is itself stored into the TTF_Fontinstance on line 1817.

So:

SDL_ttf reads everything it needs from the SDL_RWops during this call in any case.

This statement is wrong.

But there's more, as we can see at lines 1769 to 1771, 1784 to 1786 and 1793 to 1795, the SDL_RWops instance will be closed if the freesrc parameter is set to true, but only if there is an error. At no point is it closed if there are no errors!

At line 1801 though, we can see that the freesrc boolean is stored into the TTF_Font instance, which, as can be see on line 2754, is to close the SDL_RWops during the TTF_Close call that you will do when you will be done with your font!

So:

If freesrc is non-zero, the SDL_RWops will be closed before returning, whether this function succeeds or not.

This statement is wrong, the SDL_RWops will be closed only if the function did NOT succeed.

What does this mean for us?

The TTF_Font instance returned by the TTF_OpenFontRW function call is very closely linked to the SDL_RWops you passed to it. This instance will require your SDL_RWops to remain opened for as long as you keep your font opened.

Summary of what we know so far

So far, we have figured out that:

  • You can't free the buffer you linked to a SDL_RWops if you don't close the SDL_RWops before doing so. Otherwise, your SDL_RWops will become invalid.
  • You can't close a SDL_RWops you linked to a TTF_Font if you don't close the TTF_Font before doing so. Otherwise, your TTF_Font will become invalid.

Your error in all of this

Let me write back the content of your m_load_font function, simplified:

TTF_Font* m_load_font(const char* filename, int fontsize) {
    zip_stat_t filestat;
    uint64_t filetotal;
    SDL_RWops* rwop = SDL_AllocRW();

    /*
    ---
    Code to open the TTF file located in your ZIP
    ---
    */
    
    //Write data to a data holding array
    char* rwbuffer = new char[filestat.size];
    rwop = SDL_RWFromMem(rwbuffer, filestat.size);

    /*
    ---
    Code to write the content of the TTF file you opened into rwbuffer
    ---
    */

    //Load the font from the rwop
    TTF_Font* temp = TTF_OpenFontRW(rwop, 0, fontsize);
    SDL_FreeRW(rwop);
    delete[] rwbuffer;
    if (temp == NULL) {
        u_error(std::cerr, CC_ERROR_TTF, "TTF_OpenFontRw");
    }
    return temp;
}

Here, as you can see, right after opening your TTF_Font by passing rwop to TTF_OpenFontRW call, you free rwop by calling SDL_FreeRW. Your TTF_Font won't be usable if you close its SDL_RWops, that needs to be done only after you close your font when you no longer need it.

But that's not all, even if you didn't close the SDL_RWops, you delete rwbuffer, which is the buffer rwop is linked to. So even without closing the SDL_RWops, you delete the buffer that holds its content, rendering it unusable.

The solution

Making your code work

You should remove both the SDL_RWops closing line and the and buffer delete[] line, as such:

TTF_Font* m_load_font(const char* filename, int fontsize) {
    zip_stat_t filestat;
    uint64_t filetotal;
    SDL_RWops* rwop = SDL_AllocRW();

    /*
    ---
    Code to open the TTF file located in your ZIP
    ---
    */
    
    //Write data to a data holding array
    char* rwbuffer = new char[filestat.size];
    rwop = SDL_RWFromMem(rwbuffer, filestat.size);

    /*
    ---
    Code to write the content of the TTF file you opened into rwbuffer
    ---
    */

    //Load the font from the rwop
    TTF_Font* temp = TTF_OpenFontRW(rwop, 0, fontsize);
    if (temp == NULL) {
        u_error(std::cerr, CC_ERROR_TTF, "TTF_OpenFontRw");
    }
    return temp;
}

Avoiding memory leaks

With the way you organized your code, it will cause a memory leak, as you don't save your SDL_RWops anyware to close it whenever you close your font, and nor do you save rwbuffer's value as a pointer to delete it as soon as you close its corresponding SDL_RWops.

Here is a suggestion on how to fix it, though it's up to you to decide what is for best for your needs.

First, I'd change TTF_OpenFontRW(rwop, 0, fontsize) into TTF_OpenFontRW(rwop, 1, fontsize) in order to tell the font to automatically close the SDL_RWops that your TTF_Font holds as soon as you call TTF_Close to free it.

But closing the SDL_RWops won't free the buffer it was linked to. So, I'd keep track of rwbuffer's value as a pointer, to have delete[] rwbuffer placed right after you TTF_Close your TTF_Font.

Conclusion

Most of the time when you get a segfault, it means that you either access memory that you didn't allocate, or memory that you freed.

So whenever you get one in a case like yours, try commenting your close, free and delete calls to see if it fixes it.

Don't leave them commented for eternity though, it could cause memory leaks! If one of those calls is responsible for your segfault, then try to figure out why (or ask on StackOverflow if you can't figure out why, of course) and put that call that was responsible for the segfault somewhere else, where is actually belongs to.

If you use arrays, then check for indices to see if you aren't reading further than your array's size, as that can cause segfaults as well.

RedStoneMatt
  • 465
  • 3
  • 11
  • Top notch answer right there. Lots to learn in what you wrote. Well done! – whitehawk Aug 01 '23 at 05:52
  • @whitehawk Thank you! I'm glad you liked it. You seem to be the bounty setter, right? If my answer worked and suite your needs, do you happen to have the right to mark it as accepted then? I doubt that OP is still here after 3 years. Anyway, I am open to more related questions if you have some. Have a nice day! – RedStoneMatt Aug 01 '23 at 09:09
  • oOps - awarded now. Thanks again – whitehawk Aug 01 '23 at 14:26