Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rosbag reindex bugfix - seek to truncated position after broken chunk #2286

Conversation

emersonknapp
Copy link
Contributor

Fixes #2282

The truncate operation left the current f.tell() read head at the pre-truncated position, so the chunk infos that are written on closing the file started writing at this pre-truncated position, leaving dangling broken chunk data in between the last good chunk and the first file-end chunkinfo.

The truncate operation left the current `f.tell()` read head at the pre-truncated position, so the chunk infos that are written on closing the file started writing at this pre-truncated position, leaving dangling broken chunk data in between the last good chunk and the first file-end chunkinfo.

Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
@emersonknapp emersonknapp marked this pull request as ready for review October 4, 2022 23:27
@emersonknapp emersonknapp changed the title Reindex bugfix - seek to truncated position after broken chunk rosbag reindex bugfix - seek to truncated position after broken chunk Oct 4, 2022
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
@emersonknapp
Copy link
Contributor Author

Added a regression test - verified that test fails without f.seek change, passes with it.

@emersonknapp
Copy link
Contributor Author

@jacobperron @mjcarroll @sloretz a review would be much appreciated :) hopefully I have made it easy with the regression test.

This bug is a far corner case, but it's real: someone using a format-compliant parser that is reading records without using the index will find invalid records in the file, even after using the reindex command which purports to fix the file.

@peci1
Copy link
Contributor

peci1 commented Nov 7, 2022

Looks good to me, tests pass.

@mjcarroll mjcarroll self-assigned this Nov 15, 2022
@mjcarroll mjcarroll self-requested a review November 15, 2022 16:44
@mjcarroll mjcarroll merged commit 13cde59 into ros:noetic-devel Nov 22, 2022
@emersonknapp emersonknapp deleted the emersonknapp/reindex-truncate-remove-trailing-data branch November 23, 2022 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rosbag reindex outputs invalid bags when run on truncated files
3 participants