Skip to content
Snippets Groups Projects

fixed off by one error in index end calculation

Merged Bird, Robert F requested to merge rfbird/Cabana:fixing_iterator_bounds into master

Note, this could also likely be:

return Index( array_size, _num_soa+1, 0);

or similar as you prefer.

There's probably even something better that can be done, but I wanted to offer a solution rather than dump an issue on you on the way out the door!

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Good catch. Actually I think it should be this:

    std::size_t remainder = _size % array_size;
    std::size_t s = ( 0 == remainder ) ? _num_soa : _num_soa - 1;
    std::size_t i = ( 0 == remainder ) ? 0 : remainder;
    return Index( array_size, s, i );

    If the last array isn't full we want the end of the AoSoA to be one past the last filled element of that array. If the last array is full, the next full element is the first element of the next struct. This comes from the fact that we may have allocated more memory than the current size of the container (i.e. the capacity of the container is larger than the size).

    Edited by Slattery, Stuart
  • Author Maintainer

    Ye, looks good. My first solution was just to make the 2nd parameter max + 1, and accept that could ask for something one too far, relying on the fact that (hopefully!) (2,16) and (3,0) are the same. Something like:

    Index( array_size, _num_soa - 1, (_size-1) % array_size +1 );

    But I couldn't convince myself it was right, and GY convinced me the committed solution was fine.

    Either way, I'm happy with anything that works!

    Edited by Bird, Robert F
  • Bird, Robert F added 1 commit

    added 1 commit

    • af6e09b2 - swapped to Stuarts prefered (and more correct) version

    Compare with previous version

  • Awesome! Can you squash your 2 commits? Once you've done that I'll merge this.

  • Author Maintainer

    Sure. Re-writing history always makes me nervous.

  • In general I agree but in the context of a clean merge request like this I think it often makes sense.

  • Bird, Robert F added 1 commit

    added 1 commit

    • 7c3c2958 - Fixing indexing error in end() calculation

    Compare with previous version

  • mentioned in commit 514e624c

Please register or sign in to reply
Loading