Skip to content

Fix cache initialization#12127

Merged
JosiahWI merged 1 commit intoapache:masterfrom
JosiahWI:fix/cache-narrowing-conversion
Mar 26, 2025
Merged

Fix cache initialization#12127
JosiahWI merged 1 commit intoapache:masterfrom
JosiahWI:fix/cache-narrowing-conversion

Conversation

@JosiahWI
Copy link
Copy Markdown
Contributor

Fixes #12124

@bneradt tested this patch and confirmed it resolves the issue.

@JosiahWI
Copy link
Copy Markdown
Contributor Author

The Debian CI failed (bneradt reran it, thanks!) with a free() error during one of the cache_Alternate_S_to_L tests. Probably a real issue, but probably not easy to diagnose.

fd{disk->fd},
disk{disk},
_preserved_dirs{static_cast<int>(len)}
_preserved_dirs{len}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Using off_t seems makes sense because this Stripe::len is off_t.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I could also look at how big a change it would be to make the table size an off_t type, but I need to be careful because I don't understand yet why they were different in the first place. It seems like using the same type for both would be more consistent.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This has not been done yet, but I am adding it to my personal TODO.

Copy link
Copy Markdown
Contributor

@masaori335 masaori335 left a comment

Choose a reason for hiding this comment

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

Looks good.

@JosiahWI JosiahWI merged commit 090141a into apache:master Mar 26, 2025
@github-project-automation github-project-automation Bot moved this to For v10.1.0 in ATS v10.1.x Mar 26, 2025
@cmcfarlen cmcfarlen moved this from For v10.1.0 to picked v10.1.0 in ATS v10.1.x Mar 26, 2025
@cmcfarlen cmcfarlen modified the milestones: 10.2.0, 10.1.0 Mar 26, 2025
@cmcfarlen
Copy link
Copy Markdown
Contributor

Cherry-picked to 10.1.x branch

cmcfarlen pushed a commit that referenced this pull request Mar 26, 2025
bneradt pushed a commit to bneradt/trafficserver that referenced this pull request Apr 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: picked v10.1.0

Development

Successfully merging this pull request may close these issues.

10.1.0: Cache startup assertion failure

4 participants