Skip to content

Oom alloc#463

Closed
sftse wants to merge 1 commit intotafia:masterfrom
sftse:oom-alloc
Closed

Oom alloc#463
sftse wants to merge 1 commit intotafia:masterfrom
sftse:oom-alloc

Conversation

@sftse
Copy link
Copy Markdown
Contributor

@sftse sftse commented Aug 29, 2024

Was it intentional that fn from_sparse assumes cells are in (row, col) order? This test example demonstrates this is not always the case and OOM allocates due to an overflow.

Based on #462 restructured history

@sftse sftse mentioned this pull request Aug 30, 2024
@sftse
Copy link
Copy Markdown
Contributor Author

sftse commented Aug 30, 2024

Added another example that OOMs, hasn't been fixed yet, I'm working around it for my usecase by using unstable set_alloc_error_hook

@tafia
Copy link
Copy Markdown
Owner

tafia commented Sep 16, 2024

Thanks!
Tiny comment just to help future debugging if needed.

@sftse
Copy link
Copy Markdown
Contributor Author

sftse commented Sep 16, 2024

Do you mean a comment explaining the last OOM file that was added?

@sftse sftse force-pushed the oom-alloc branch 2 times, most recently from 17fc324 to 1ffc325 Compare October 27, 2024 13:24
@sftse
Copy link
Copy Markdown
Contributor Author

sftse commented Oct 27, 2024

I've added a FIXME to the function call that OOMs, if this was not what was meant please advise further.
This is ready to merge imho.

@sftse
Copy link
Copy Markdown
Contributor Author

sftse commented Oct 28, 2024

Curious whether you know anything about the unknown record types as mentioned in #462.
The commit history was starting to look complex, so I folded it into this PR.

Comment thread tests/test.rs Outdated
Comment thread src/xls.rs Outdated
Comment on lines +493 to +495
// tests/high_byte_string.xls contains a record type that
// cannot be found in the "By Number" 2.3.2 table
0x00D6 => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tag 0x00D6 is RSTRING, i.e., rich multi-format strings.

Copy link
Copy Markdown
Contributor Author

@sftse sftse Jul 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reference we can link?

Copy link
Copy Markdown
Collaborator

@jmcnamara jmcnamara Jul 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem to be mentioned in the Microsoft online docs (I have an old paper copy of the "Excel Developer's Kit" and it is in that. Next best would be to link to the OpenOffice docs for the "Microsoft Excel File Format": https://www.openoffice.org/sc/excelfileformat.pdf

Comment thread src/lib.rs Outdated
Comment thread src/xls.rs Outdated
This was referenced Jul 6, 2025
@sftse sftse closed this Jul 14, 2025
@sftse sftse deleted the oom-alloc branch September 10, 2025 10:32
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.

3 participants