Skip to content

Conversation

@jrtc27
Copy link
Contributor

@jrtc27 jrtc27 commented Jan 9, 2026

On ILP32 and LP64 systems, unsigned long is sufficient to round-trip pointers, but on LLP64 systems it is too small, and on systems like CHERI where pointers are capabilities, it does not propagate capability metadata, and casting back to a pointer to yield a pointer that cannot be dereferenced.

Instead, cast via uintptr_t to ensure we always use an integral type that can losslessly round-trip pointers.

On ILP32 and LP64 systems, unsigned long is sufficient to round-trip
pointers, but on LLP64 systems it is too small, and on systems like
CHERI where pointers are capabilities, it does not propagate capability
metadata, and casting back to a pointer to yield a pointer that cannot
be dereferenced.

Instead, cast via uintptr_t to ensure we always use an integral type
that can losslessly round-trip pointers.
@coderabbitai
Copy link

coderabbitai bot commented Jan 9, 2026

Walkthrough

A single macro UNCONST(a) in src/common.h was updated to use uintptr_t instead of unsigned long for casting, improving portability and width-safety when converting const void pointers to void pointers.

Changes

Cohort / File(s) Summary
Macro cast type update
src/common.h
Updated UNCONST(a) macro cast from unsigned long to uintptr_t for width-safe pointer conversion

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: replacing unsigned long with uintptr_t in the UNCONST macro cast.
Description check ✅ Passed The description clearly explains the rationale for the change, detailing why uintptr_t is more appropriate than unsigned long across different architectures.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cd0b269 and f5d3a4c.

📒 Files selected for processing (1)
  • src/common.h
🔇 Additional comments (1)
src/common.h (1)

58-58: Excellent portability fix!

This change correctly addresses critical issues on LLP64 and CHERI systems. Using uintptr_t instead of unsigned long ensures:

  • LLP64 (Windows x64): Prevents pointer truncation (where unsigned long is 32 bits but pointers are 64 bits)
  • CHERI capability systems: Preserves capability metadata during the cast
  • Standards compliance: uintptr_t is the C99/C11 standard type designed specifically for lossless pointer-to-integer round-trips

The required <stdint.h> header is already included (Line 35), so the change is safe to deploy immediately.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@rsmarples rsmarples merged commit 2c1db04 into NetworkConfiguration:master Jan 9, 2026
17 checks passed
@rsmarples
Copy link
Member

Thanks!

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.

2 participants