-
Notifications
You must be signed in to change notification settings - Fork 579
Encode the misc SV_* flags using bitshifts rather than decimal integers #19120
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
Conversation
This is better than the decimals, but I'd find it easier to read if they were just hex constants (as most of the other flags are, even in sv.h). Generally the only reason I look at these values is because I'm sitting in gdb trying to check what set of flags a given number corresponds to. It's not a huge effort to convert the shifts into hex mentally, but still more than none. |
On 9/12/21 5:10 PM, Hugo van der Sanden wrote:
This is better than the decimals, but I'd find it easier to read if they
were just hex constants (as most of the other flags are, even in sv.h).
Generally the only reason I look at these values is because I'm sitting
in gdb trying to check what set of flags a given number corresponds to.
It's not a huge effort to convert the shifts into hex mentally, but
still more than none.
I prefer the way the P.R. is currently written.
I use these to see which bit is set, and to see how many bits are in use
to see if anything is available. This gives that info at a glance. I
don't find the mental conversion into hex problematic
|
The goal was:
more visible than decimal... So write them as hex?
This missing one is more obvious than decimal? |
On Mon, 13 Sept 2021 at 01:10, Hugo van der Sanden ***@***.***> wrote:
This is better than the decimals, but I'd find it easier to read if they
were just hex constants (as most of the other flags are, even in sv.h).
Generally the only reason I look at these values is because I'm sitting in
gdb trying to check what set of flags a given number corresponds to. It's
not a huge effort to convert the shifts into hex mentally, but still more
than none.
Cant you just put the hex value next to the bitshift form in a comment?
Then we can have our cake and eat it too.
Yves
…--
perl -Mre=debug -e "/just|another|perl|hacker/"
|
Is there a reason now to not merge this? |
Using decimal integers is confusing, so we show the hex and decimal values as a comment. Note: Yves edited the commit message and added the commented values.
9400eaa
to
5f51c4d
Compare
I have rebased this and modified it to show the hex and decimal values as a comment, and force pushed it. I also decided to "document" the unused bit with a define SV_FLAG_BIT3_UNUSED. I will merge this once it passes CI. |
On Tue, Feb 07, 2023 at 11:00:14PM -0800, Yves Orton wrote:
I have rebased this and modified it to show the hex and decimal values
as a comment, and force pushed it. I also decided to "document" the
unused bit with a define SV_FLAG_BIT3_UNUSED.
Personally I'm not a fan of this approach for unused bits.
Leaving a blank line holding just a comment like /* unused */ stands out
(to me, anyway), while a solid block of defines looks like they're all in
use, and you'd have to look carefully (and know to look) to spot that
SV_FLAG_BIT3_UNUSED is actually an unused bit. Then you'd have to worry
whether such a define is indicating an unused bit, or whether it's actually
used (or was used) as a flag to some sv_foo() function to indicate
something about skipping a bit when processing the SV.
…--
A major Starfleet emergency breaks out near the Enterprise, but
fortunately some other ships in the area are able to deal with it to
everyone's satisfaction.
-- Things That Never Happen in "Star Trek" #13
|
Its not a hill I want to die on, so I don't mind removing it. But the contrary view is the it is much easier to manage in terms of ensuring things are well formed, and automating things like showing the value different ways. With a "gap" its harder for a tool to see that the defines are all related, and things like that. I think if we have a well defined convention, then it should be pretty obvious its not a real bit flag. Maybe SV_FLAG_BIT3_UNUSED isnt perfect. It could be: 'XXXXXXX_SV_B3' or something like that. Its easier to teach humans a convention than it is to teach a program about human irregularity. I would like us to automate more and more of our maintenance. I see lists of defines where the first K have N characters, then the next K have N+1, etc, which IMO makes it hard to read. The more regular we are about these lists the more reasonable it is to write a script that keeps them well formed or to detect when they are not. |
The bitwise pattern, and the gap at (1<<3), are more visible this way.