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

Loading Surface from file sometimes fails due to invalid file name string type #40

Closed
kr41 opened this Issue May 1, 2015 · 3 comments

Comments

Projects
None yet
2 participants
@kr41
Copy link
Contributor

kr41 commented May 1, 2015

Loading Surface from file sometimes doesn't work.

    /**
     * Load from filename. If any data is already stored, the data will be freed.
     */
    @nogc
    bool loadFromFile(string filename) nothrow {
        import std.file : exists;

        immutable bool ex = exists(filename);
        if (!ex) {
            print_fmt("File %s does not exists.\n", filename.ptr);
            return false;
        }

        SDL_FreeSurface(_surface); // free old surface

        _surface = IMG_Load(filename.ptr);
        if (!_surface) {
            print_fmt("Could not load image %s. Error: %s.\n", filename.ptr, SDL_GetError());
            return false;
        }

        assert(_surface.pixels, "Invalid pixel data.");

        return true;
    }

Take note, that filename.ptr is passed to IMG_Load function. IMG_Load is C function, so it operates zero-terminated strings. But D string is not zero-terminated. So if memory for a D string is allocated without trailing zero, loadFromFile() will not work. Here is the test:

    string filenameWithTail = "samples/images/XS.png;trailing chars goes here";
    string cleanFilename = filenameWithTail[0..21];
    writefln("Clean filename is '%s'", cleanFilename);
    Surface invalidSurface = Surface(cleanFilename);

The output will be:

Clean filename is 'samples/images/XS.png'
Could not load image samples/images/XS.png;trailing chars goes here. Error: Couldn't open samples/images/XS.png;trailing chars goes here.

It usually works, because D compiler puts zero at the end of static strings. But if you get string from file, it won't. The fast fix is to use toStringz function from std.string module:

        _surface = IMG_Load(filename.toStringz);

But it doesn't work with @nogc declaration of loadFromFile(). So I don't know how to fix it and keep this declaration.

@kr41 kr41 changed the title Loading Surface from file sometimes failed due to invalid file name string type Loading Surface from file sometimes fails due to invalid file name string type May 1, 2015

@Dgame

This comment has been minimized.

Copy link
Owner

Dgame commented May 1, 2015

I've never had this issue but I will try to fix this even if it is very unlikely to happen. But the phobos version of toStringz is evil, It wasted your memory.

Dgame added a commit that referenced this issue May 1, 2015

@Dgame

This comment has been minimized.

Copy link
Owner

Dgame commented May 1, 2015

I made my own toStringz method and fix all occurrences I've found. Thanks for reporting. ;)

@Dgame Dgame closed this May 1, 2015

@kr41

This comment has been minimized.

Copy link
Contributor Author

kr41 commented May 1, 2015

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.