Skip to content

Conversation

@mikolaj-pirog
Copy link
Member

@mikolaj-pirog mikolaj-pirog commented Oct 24, 2025

Typed enums are c23 features and are too new to be used. This PR restores the types in the __processor_model struct back to unsigned int, removes typed enums, and uses the enum in the function as a variable that's later assigned to a global in order to prevent errors fixed initially here: #164713

See #165034 for more background

if (Type != CPU_TYPE_MAX)
__cpu_model.__cpu_type = Type;
if (Subtype != CPU_SUBTYPE_MAX)
__cpu_model.__cpu_subtype = Subtype;
Copy link
Member

Choose a reason for hiding this comment

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

Likewise

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't follow, I think one of your comments wasn't posted

Copy link
Member

Choose a reason for hiding this comment

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

Ugh, silly GitHub. I was thinking early return might be useful here.

Comment on lines +680 to +684
if (Type != CPU_TYPE_MAX)
__cpu_model.__cpu_type = Type;
if (Subtype != CPU_SUBTYPE_MAX)
__cpu_model.__cpu_subtype = Subtype;

Copy link
Contributor

@e-kud e-kud Oct 25, 2025

Choose a reason for hiding this comment

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

I won't block the PR but I don't like this kind of refactor. Previously it was a pure static function that touched no context. Now it implicitly modifies the global variable. References to __cpu_type and __cpu_subtype were additional return values.

Copy link
Member Author

Choose a reason for hiding this comment

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

I understand the downsides, but I believe in this file it's not gonna be a problem, especially how the function is local.

I can refactor later to not use the global like that, but the previous solution of passing a pointer to some fields of the global isn't much different than what the code is doing now

Copy link
Member

Choose a reason for hiding this comment

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

I commented on my original PR that passing pointers to field while assuming a different signedness is not good. What about passing pointers to entire struct? That would required the struct to be named but I think we should be fine by that?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, passing the pointer to the struct would be my preference as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment