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

Introduce countDigits() function for PID and UID field widths #1561

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Explorer09
Copy link
Contributor

The countDigits function was proposed in #1270 (@BenBE and @MrCirdo). And I make PID and UID field widths the first users of this new function.

Explorer09 and others added 2 commits November 12, 2024 01:29
The GPU_TIME field is now fixed width, and thus there is no need to
call Row_updateFieldWidth().

Signed-off-by: Kang-Che Sung <[email protected]>
countDigits() is designed with 64-bit integer input and may be reused
for computing field widths of, for example, memory addresses.

Signed-off-by: Kang-Che Sung <[email protected]>
Co-authored-by: Benny Baumann <[email protected]>
Co-authored-by: Odric Roux-Paris <[email protected]>
@BenBE BenBE added enhancement Extension or improvement to existing feature code quality ♻️ Code quality enhancement labels Nov 11, 2024
@BenBE BenBE added this to the 3.4.0 milestone Nov 11, 2024
/* Counts the number of digits needed to print "n" with a given base.
If "n" is zero, returns 1. This function expects small numbers to
appear often, hence it uses a O(log(n)) time algorithm. */
size_t countDigits(size_t n, size_t base) {
Copy link
Contributor Author

@Explorer09 Explorer09 Nov 13, 2024

Choose a reason for hiding this comment

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

This question is a bit pedantic here. Since we plan to use countDigits for memory address, too, the more proper type for this function's input would be uintptr_t and not size_t. May I ask, is it better for this function's type be changed to uintptr_t to better indicate the intent? The output type can remain size_t as I see no reason for changing it.

Technical thing here: size_t and uintptr_t do not necessarily have the same size. In the C standard's sense, sizeof(size_t) <= sizeof(uintptr_t). On flat memory systems they are indeed the same size, but on other platforms (e.g. segmented memory) size_t can be defined to be narrower than uintptr_t.

Copy link
Member

Choose a reason for hiding this comment

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

Use uint64_t and there's no problem (except for some potential upcasts for clang).

But I don't think we need to change this here; size_t is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality ♻️ Code quality enhancement enhancement Extension or improvement to existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants