3

I have a fairly simple log() method for a GL shader and program convenience classes, since the respective compile and link methods only return a bool, while hiding all the GL calls; e.g.,

std::string
glShader::log () const
{
    std::string info;
    GLint len = 0;

    if (!glIsShader(gl_shader_obj))
        info = "(invalid shader object)\n";
    else
        glGetShaderiv(gl_shader_obj, GL_INFO_LOG_LENGTH, & len);

    if (len != 0)
    {
        info.resize(static_cast<std::string::size_type>(len));
        glGetShaderInfoLog(gl_shader_obj, len, NULL, & info[0]);
    }

    return info;
}

Is this a misuse of the std::string::resize (size_type) argument? I known that C++11 mandates a null terminating character when queried, i.e., c_str(); but does it guarantee its presence in the storage? This might be a 'reasonable' way to implement string to simplify C string access, but not a requirement.

However, GL_INFO_LOG_LENGTH includes \0 in the number of characters in the log, provided there is a log; otherwise the log length is simply zero.

Am I potentially writing past the end of the string's reserved buffer in this fashion? Should I be using (len - 1) in the InfoLog call? Or do I have the idea about C++11 strings wrong? That is, can I safely overwrite with the null terminator?

Brett Hale
  • 21,653
  • 2
  • 61
  • 90

1 Answers1

4

Yes, this is a valid use of std::string. You can get a pointer to the first character and write to the array of characters, so long as you don't exceed the range [0, size()).

However, you did make one mistake. See, GL_INFO_LOG_LENGTH includes the NUL terminator character in the length. Which means that, technically speaking, your std::string is one character longer than it needs to be. The last character of info will be a NUL character, and the std::string will treat that like it is part of the string's data, rather than a delimiter marking the end of the string.

You should not try to fix this by subtracting 1 from len before setting info's size. Why? Because glGetShaderInfoLog will always NUL terminate the string it writes. So if you shrink len, it will chop off the last actual character from the log.

Instead, you should shrink info after you've copied it from OpenGL:

info.resize(static_cast<std::string::size_type>(len));
glGetShaderInfoLog(gl_shader_obj, len, NULL, & info[0]);
info.pop_back();

In C++17, the standard wording has changed to allow writing to the NUL-terminator of a string, so long as you are overwriting it with a NUL character. It also allows string::data to return a non-const character array. Therefore, this will work fine now:

info.resize(static_cast<std::string::size_type>(len - 1));
glGetShaderInfoLog(gl_shader_obj, len, NULL, info.data());

The standard requires that info will have size+1 bytes in it.

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
  • I overlooked the fact that `InfoLog` NUL terminates regardless. So the `end()` iterator yields the 'real' `\0`, and the prefix `--` yields the position of my 'false' `\0` that I want to be rid of, right? – Brett Hale Feb 04 '16 at 16:02
  • @BrettHale: No, `end()` returns an iterator one-past-the-end of the string. You are *not* allowed to assume that `string::end()` is dereferencable and that it will return the NUL character. To put it a different way, the fact that `std::string` is NUL terminated is a hidden detail. The code I wrote above is just snipping off the last character, which we know is a NUL character written by `glGetShaderInfoLog`. It has nothing to do with `string`'s own internal NUL terminator. – Nicol Bolas Feb 04 '16 at 16:09
  • Of course. Forgetting iterator ranges are half-open... I'm tying myself in knots here. Also wondering if the `pop_back` method would be useful. Thanks for clearing things up. – Brett Hale Feb 04 '16 at 16:14
  • @NicolBolas: "To put it a different way, the fact that std::string is NUL terminated is a hidden detail." Is it actually though? I mean if `std::string` supports `c_str` in any reasonable way (that doesn't involve suddenly making a copy) then the NUL really must be there, no? I suppose its possible that `string::end` is not exactly what you might expect... I mean you definitely make your code less generic if not outright wrong if you ever treat an `end` iterator as dereferenceable. – Chris Beck Feb 05 '16 at 06:24
  • I guess it was discussed at length [here](http://stackoverflow.com/questions/7554039/is-stringc-str-no-longer-null-terminated-in-c11) some years ago – Chris Beck Feb 05 '16 at 06:35
  • @ChrisBeck: "*Is it actually though?*" Who cares? The standard clearly says that dereferencing the `end` iterator yields undefined behavior. – Nicol Bolas Feb 05 '16 at 14:14