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

Update system/SystemPacketBuffer to use uint32_t(instead of uint16_t) for several fields to accommodate large payloads #31115

Closed
pidarped opened this issue Dec 19, 2023 · 3 comments · Fixed by #31776
Assignees

Comments

@pidarped
Copy link
Contributor

Feature description

Currently, SystemPacketBuffer uses uint16_t for a lot of its internal and public parameters. These would need to be updated to accommodate large payload sizes when operating over TCP.
Internally, fields such as alloc_size uses uint16_t.
APIs such as MaxDataLength() and AvailableDataLength() return uint16_t. These need to use uint32_t when handling large sized buffers that go beyond uint16_t limits.

Platform

core

Platform Version(s)

No response

Anything else?

No response

@bzbarsky-apple
Copy link
Contributor

Shouldn't it just use size_t for the APIs?

@pidarped
Copy link
Contributor Author

@bzbarsky-apple, I had considered using size_t but on second thoughts felt that uint32_t might be better. Since the max size of the field in the TCP payload is set to 4 bytes, everything in the code, from processing to storage, should be dictated by that, IMO.
Changes have to be made to both the API arguments as well as internal storage. Using uint32_t at all the relevant places seems to require less static_casts. Also, size_t, being not the same in all architectures(8 bytes in Linux standalone) might pose weird problems(during casting in and out of it) in some edge cases, that I would want to avoid.

@pidarped
Copy link
Contributor Author

pidarped commented May 5, 2024

Fixed by #33001 in master.

@pidarped pidarped closed this as completed May 5, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in [Feature] TCP May 5, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in [Device Type] Cameras May 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Archived in project
2 participants