Skip to content

Fix null dereference when building a texture from memory - #112

Merged
zao merged 2 commits into
PathOfBuildingCommunity:masterfrom
hard25670559:fix/texture-from-memory
Aug 15, 2026
Merged

Fix null dereference when building a texture from memory#112
zao merged 2 commits into
PathOfBuildingCommunity:masterfrom
hard25670559:fix/texture-from-memory

Conversation

@hard25670559

Copy link
Copy Markdown
Contributor

Problem

r_tex_c(r_ITexManager*, std::unique_ptr<image_c>, int) assigns the mip set to its parameter rather than to the member PerformUpload reads:

r_tex_c::r_tex_c(r_ITexManager* manager, std::unique_ptr<image_c> img, int flags)
{
    Init(manager, {}, flags);

    // Direct upload
    img = BuildMipSet(std::move(img));   // <- parameter, not the member
    PerformUpload(this);                 // <- reads tex->img, still null
}

PerformUpload then does tex->Upload(*tex->img, tex->flags) on a null img, so every use of this constructor crashes. There appear to be no in-tree callers, which is presumably why it has gone unnoticed.

Change

Assign to the member.

Verification

Found while building textures from memory in a downstream branch: the constructor crashed with EXC_BAD_ACCESS at r_tex_c::Upload on the first call, and worked after this one-line change. Existing texture loading paths go through LoadFile/Upload directly and are untouched.

The image constructor assigned the mip set to its parameter rather than
to the member that PerformUpload reads, so `img` was still null by the
time Upload dereferenced it. Any call to this constructor crashed.

Assign to the member instead.
Renamed the parameter to avoid disambiguation in the first place.
Comments should preferably document the state/intent of the code while commit messages cover rationale.
@zao
zao merged commit b04f28f into PathOfBuildingCommunity:master Aug 15, 2026
1 check passed
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.

2 participants