-
Notifications
You must be signed in to change notification settings - Fork 615
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
Fix parsing numpy header #4903
Fix parsing numpy header #4903
Conversation
Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
!build |
CI MESSAGE: [8610899]: BUILD STARTED |
ndims = list(range(33)) | ||
with tempfile.TemporaryDirectory(prefix=gds_data_root) as test_data_root: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need to test all of them? Wouldn't a small subset like 1, 2, 3, 7, 16, 32
(or something more informed) do? Of course, if it doesn't affect the test time in a substantial way, we can keep all of them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The most interesting case is 21, as that has minimal padding after the header, which makes it sensitive to small offset errors. But the test is very simple and lightweight so I went for all the cases.
CI MESSAGE: [8610899]: BUILD PASSED |
* Fix offset at which a null character is added in the header buffer * Add a test aimed to catch such bugs in header reading and parsing --------- Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
* Fix offset at which a null character is added in the header buffer * Add a test aimed to catch such bugs in header reading and parsing --------- Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
Category:
Bug fix (non-breaking change which fixes an issue)
Description:
In #4897, I moved the calculation of the header start pointer (
header = token_mem.get() + token_len;
) down in the function, but setting null-termination character stayed higher, which means that the\0
is settoken_len
bytes too early.Additional information:
Affected modules and functionalities:
Key points relevant for the review:
Tests:
Checklist
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: N/A