Conversation
The enum `exire_id` fits easily into a uint8_t. Use that type for storing expire timeout list indices.
bagder
approved these changes
Aug 15, 2026
There was a problem hiding this comment.
Pull request overview
This PR reduces the storage size of per-easy-handle timeout list indices by changing expire_timers.next[] and expire_timers.first from expire_id to uint8_t, since EXPIRE_LAST fits in a byte. This helps shrink struct expire_timers and the per-handle timeout bookkeeping footprint in the multi interface.
Changes:
- Change
struct expire_timerslist linkage fields (next[],first) touint8_t. - Update timeout list iteration and validation logic in
lib/multi.cto useuint8_tindices. - Adjust timer tracing iteration in
lib/curl_trc.cand parameter naming inlib/multiif.h.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/urldata.h | Shrinks timeout list linkage storage to uint8_t. |
| lib/multiif.h | Renames Curl_expire_clear parameter for consistency (id → eid). |
| lib/multi.c | Updates timeout list walk/insert/remove code for byte-sized indices. |
| lib/curl_trc.c | Updates verbose timer tracing loop variable to uint8_t. |
Suppressed comments (2)
lib/multi.c:3712
- timeouts->next is now a uint8_t indexed by the expire id, but this code still writes to timeouts->next[eid] (and does it twice). If an out-of-range expire_id is ever passed, the current cast-based check can wrap and this becomes an out-of-bounds write. Use a range check on eid before casting, and index next[] via the validated uint8_t id, with a single assignment.
}
timeouts->next[eid] = *anchor;
timeouts->next[eid] = *anchor;
*anchor = id;
lib/multi.c:3737
- Curl_expire() performs its argument validation by first narrowing eid to uint8_t and then comparing. That allows values > 255 to wrap and evade the check. Validate eid before any narrowing (or just validate eid directly here, since the uint8_t copy is otherwise unused).
uint8_t prev_id = timeouts->first, id = (uint8_t)eid;
struct curltime set;
if(id >= EXPIRE_LAST) {
DEBUGASSERT(0);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
bagder
approved these changes
Aug 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The enum
exire_idfits easily into a uint8_t. Use that type for storing expire timeout list indices.