Page MenuHome
Differential D4178

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

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

Details

Reviewers
Bastien Montagne (mont29)
Campbell Barton (campbellbarton)
Brecht Van Lommel (brecht)
Maniphest Tasks
T60261: Crash when pasting a material in 2.79 file

Diff Detail

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

Event Timeline

Philipp Oeser (lichtwerk) created this revision.Jan 7 2019, 12:26 PM
Harbormaster completed remote builds in B2707: Diff 13134.Jan 7 2019, 12:26 PM
Philipp Oeser (lichtwerk) mentioned this in T60261: Crash when pasting a material in 2.79 file.Jan 7 2019, 12:27 PM
Brecht Van Lommel (brecht) requested changes to this revision.Jan 7 2019, 12:49 PM
Brecht Van Lommel (brecht) added a subscriber: Brecht Van Lommel (brecht).
Brecht Van Lommel (brecht) added inline comments.
source/blender/blenkernel/intern/material.c
1400

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

1431

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
Philipp Oeser (lichtwerk) updated this revision to Diff 13238.Jan 16 2019, 2:34 PM

review by brecht
also add proper update for preview generation

Harbormaster completed remote builds in B2744: Diff 13238.Jan 16 2019, 2:34 PM
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...

Bastien Montagne (mont29) mentioned this in rB0a378b8ebce4: Fix BKE_id_copy_ex() being able to 'return' garbage in copied ID..Jan 16 2019, 4:16 PM
Bastien Montagne (mont29) added a comment.Jan 16 2019, 4:17 PM

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) accepted this revision.Jan 16 2019, 4:22 PM
Brecht Van Lommel (brecht) added inline comments.
source/blender/blenkernel/intern/material.c
1403–1405

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
Brecht Van Lommel (brecht) added a comment.Jan 16 2019, 4:23 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.

Campbell Barton (campbellbarton) closed this revision.Jan 16 2019, 10:59 PM

Committed rBe57ee5934a30e87410b7544ba8d18054881b2c00

Revision Contents
Changeset List

PathSize
source/
blender/
blenkernel/
intern/
7 lines

Diff 13134

View Options

source/blender/blenkernel/intern/material.c

Loading...