Page MenuHome

Fix T60261: Crash when pasting a material without a nodetree
ClosedPublic

Authored by Philipp Oeser (lichtwerk) on Jan 7 2019, 12:26 PM.

Diff Detail

Repository
rB Blender
Branch
T60261 (branched from master)
Build Status
Buildable 2744
Build 2744: arc lint + arc unit

Event Timeline

Brecht Van Lommel (brecht) requested changes to this revision.Jan 7 2019, 12:49 PM
Brecht Van Lommel (brecht) added inline comments.
source/blender/blenkernel/intern/material.c
1402–1407

if (ma->nodetree) seems better, no need to lose the node tree if it's there but unused.

1433–1435

The same bug exists here. If matcopybuf.nodetree is NULL, then ntreeCopyTree_ex returns an uninitialized variable.

To me it seems it's really BKE_id_copy_ex that should be fixed, since I can imagine similar bugs in other places.

This revision now requires changes to proceed.Jan 7 2019, 12:49 PM

review by brecht
also add proper update for preview generation

Philipp Oeser (lichtwerk) marked an inline comment as done.Jan 16 2019, 2:34 PM

regarding the handling in BKE_id_copy_ex, I would like to get @Bastien Montagne (mont29) on board...

Indeed behavior of BKE_id_copy_ex was dangerous here, since it accepts NULL source ID, it should ensure returned new copy is properly initialized.

Done in rB0a378b8ebce46

Brecht Van Lommel (brecht) added inline comments.
source/blender/blenkernel/intern/material.c
1405–1407

Setting matcopybuf.nodetree = NULL; is redundant, so could be left out.

This revision is now accepted and ready to land.Jan 16 2019, 4:22 PM

The patch looks fine too. Even if @Bastien Montagne (mont29)'s fix avoids the problem, it's still good not to assume ntreeCopyTree_ex accepts NULL.