Fix sizing of result array when calling GetScenePrimPaths with some invalid instance indices #2960
Conversation
…nvalid instance indices. We always need the result to be the size of the original request array, not the size of the number of valid entries in the array. This fixes crashes that could occur because the entries in the "instance index to result vector index" map expect the result entries to line up with the entries in the original request, and thus would operate past the end of the array.
|
Filed as internal issue #USD-9333 |
|
Hopefully @mtavenrath agrees this is the right thing to do? |
| @@ -2674,7 +2674,7 @@ UsdImagingInstanceAdapter::GetScenePrimPaths( | |||
| } | |||
| } | |||
|
|
|||
| result.resize(validIndices); | |||
| result.resize(instanceIndices.size()); | |||
There was a problem hiding this comment.
If my understanding of the PR description is correct the result vector has to have the correct size always and thus it has to be resized earlier. Good places are line 2638 or line 2657. Is 0 a good default when passing invalid indices?
There was a problem hiding this comment.
You mean to match the result size with the inputIndices size even when there are no valid indices? That's a good point. I suspect it is my commit message that is erroneous here, and returning a completely empty result vector is probably okay too in this situation? Though I'm also happy to move the resize call up higher if there is general agreement that the output vector size should match the input vector size always.
I'm not sure what you mean by "Is 0 a good default when passing invalid indices?" Is zero a good default for what?
There was a problem hiding this comment.
Yes, if the result vector must match the size of the input indices this should be true even if all indices are invalid.
I'm not sure what you mean by "Is 0 a good default when passing invalid indices?" Is zero a good default for what?
Was was under the wrong impression that result contains integer value. Since it contains paths the default value is good and the result should be resized before valid indices.
…ngth as the input instanceIndices vector even if none of the instance indices are valid. This guarantee should ease the burden of error testing on callers of this method.
|
On the suggestion of @mtavenrath, moved the code to resize the result vector so that it is set to the right length even in the case where no valid instance indices are provided. |
Description of Change(s)
Fix sizing of result array when calling GetScenePrimPaths with some invalid instance indices. We always need the result to be the size of the original request array, not the size of the number of valid entries in the array. This fixes crashes that could occur because the entries in the "instance index to result vector index" map expect the result entries to line up with the entries in the original request, and thus would operate past the end of the array.