Skip to content

Conversation

@MarkWangChinese
Copy link
Contributor

Change net_buf_max_len to use size_t as return value type to align with the rest of the net_buf APIs.

@zephyrbot zephyrbot added area: Networking Buffers net_buf/net_buf_simple API & implementation area: Networking labels Oct 23, 2025
@jhedberg
Copy link
Member

@MarkWangChinese since you decided to open a new PR for this instead of keeping it in the other PR (and moving everything else out) it means that this will not fulfill the review time criteria before the feature freeze. Because of that it's essential to highlight that it's an API consistency fix. You're also missing the update to the migration guide as we discussed.

@henrikbrixandersen henrikbrixandersen self-requested a review October 23, 2025 20:55
Copy link
Member

@henrikbrixandersen henrikbrixandersen left a comment

Choose a reason for hiding this comment

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

This needs to go through the breaking API change process, right?

@henrikbrixandersen henrikbrixandersen added the Breaking API Change Breaking changes to stable, public APIs label Oct 23, 2025
@jhedberg
Copy link
Member

This needs to go through the breaking API change process, right?

IMO "breaking" is a too strong word here. The underlying storage is still uint16_t so there will not be any information loss, even if the return value is stored in a uint16_t. Worst case you'd get compiler or static analyzer warnings of implicit cast to smaller integer type, and for that I think a migration guide entry is sufficient. Any comments from @carlescufi? (author of the change process)

@MarkWangChinese
Copy link
Contributor Author

@MarkWangChinese since you decided to open a new PR for this instead of keeping it in the other PR (and moving everything else out) it means that this will not fulfill the review time criteria before the feature freeze. Because of that it's essential to highlight that it's an API consistency fix. You're also missing the update to the migration guide as we discussed.

Thanks. I want to keep the #97440 to continue talking about large size support. Maybe creating another RFC to talk about it is better? Since the "breaking API" need to be clarified too, I think this change can't be merged before feature freeze.

@carlescufi
Copy link
Member

I agree with @jhedberg here. I would not consider this a breaking change. Worst case scenario you get an explicit warning, and given that the internal storage is still 16-bit, an overflow can never happen, so there is zero risk. I would agree to merge this with an entry in the migration document.

Change net_buf_max_len to use size_t as return value type to align with
the rest of the net_buf APIs.

Signed-off-by: Mark Wang <yichang.wang@nxp.com>
@MarkWangChinese MarkWangChinese force-pushed the feature/align_net_buf_return_value branch from 5da44f3 to 6fd0398 Compare November 19, 2025 07:18
@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Networking Buffers net_buf/net_buf_simple API & implementation area: Networking Breaking API Change Breaking changes to stable, public APIs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants