Overall this code indeed is probably unreachable; I got rid of most of unique_ptr things in the rest of OBJ importer (where it was even using them incorrectly to begin with), and my impression is that the unique_ptr usage in MTL importer part is also "a bit strange" - I don't quite see what advantages it brings, and is fairly confusing to read/understand.

This revision is now accepted and ready to land.Sep 1 2022, 6:07 AM

In D15827#430143, @Aras Pranckevicius (aras_p) wrote:

Looks good.

Overall this code indeed is probably unreachable; I got rid of most of unique_ptr things in the rest of OBJ importer (where it was even using them incorrectly to begin with), and my impression is that the unique_ptr usage in MTL importer part is also "a bit strange" - I don't quite see what advantages it brings, and is fairly confusing to read/understand.

Thanks.

Actually, I ran into this issue because I am working on changes related to T69169: Improve handling of embedded data-blocks (root nodetrees and master collections), which among other changes will result in the embedded node tree to be immediately assigned to its owner ID, removing any reason to use a unique_ptr here. So we should get rid of it hopefully soon in master. ;)

Closed by commit rB414baf9780d9: Fix invalid memory handling in C++ OBJ MTL Importer. (authored by Bastien Montagne (mont29)). · Explain WhySep 1 2022, 10:34 AM

This revision was automatically updated to reflect the committed changes.

Bastien Montagne (mont29) added a commit: rB414baf9780d9: Fix invalid memory handling in C++ OBJ MTL Importer..

Aras Pranckevicius (aras_p) mentioned this in rBfa4001300946: Cleanup: obj: simplify material node tree creation.Sep 1 2022, 8:27 PM