Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

glfwGetError should accept a pointer to a cstring instead of a cstringarray #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zacharycarter
Copy link

@zacharycarter zacharycarter commented Sep 17, 2021

The docs for GLFW indicate that this function should accept as a parameter:

[in] description Where to store the error description pointer, or NULL.

It also states that:

The returned string is allocated and freed by GLFW. You should not free it yourself. It is guaranteed to be valid only until the next error occurs or the library is terminated.

I think there's a bit of a inaccuracy in that last part of the documentation, because a string isn't returned it's just allocated by GLFW and the pointer is made to point at it.

If we take a look at some of the example GLFW code itself, we can see that it is indeed the case that a cstring's address is intended to be passed to the function:

int main(int argc, char** argv)
{
    int xpos, ypos, height;
    const char* description;
    GLFWwindow* windows[4];


    if (!glfwInit())
    {
        glfwGetError(&description);
        printf("Error: %s\n", description);
        exit(EXIT_FAILURE);
    }
...

So this PR just addresses this. I'm not sure how much it potentially breaks for users.

@zacharycarter
Copy link
Author

zacharycarter commented Sep 17, 2021

Oh - I realized that this file was generated just now haha... Well, I guess the PR needs to be a bit more elaborate than this. Let me try to track down where to make the change. If I can figure it out in five minutes or so I'll do it, otherwise I'll leave it to you, unless you really want help with it :)

Edit:

Okay yeah, I'm not going to touch it anymore - I'm just going to fix it in my code for now. I see there's multiple places of cstringarray and probably some where it actually needs to be present, however it's only referenced once in the generator, so I will leave it up to you to decide how to fix this, if you want to even :)

Feel free to close this! I just wanted to leave it here until you saw it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant