htop icon indicating copy to clipboard operation
htop copied to clipboard

darwin/PlatformHelpers: fix CPU detection for PowerPC

Open barracuda156 opened this issue 2 years ago • 3 comments

Closes: https://github.com/htop-dev/htop/issues/1382

Notice, that hw.cpusubtype outputs a numerical code of a CPU. For PowerPC, 100 = G5, 11 = ppc7450, 10 = ppc7400.

P. S. As far as I can see, the resulting string is only used on Apple Silicon, and in other cases it is either something (cpu is known) or unknown. It is desirable to avoid a bogus warning on PowerPC – we know those cpus very well :)

barracuda156 avatar Jan 18 '24 05:01 barracuda156

@BenBE Please have a look.

barracuda156 avatar Jan 28 '24 11:01 barracuda156

So effectively with this patch, instead of say "Intel i7-7590K" you now have "100" as the brand string? I think that might look a bit surprising and I think from an UX perspective having real strings there would be better.

BenBE avatar Jan 28 '24 20:01 BenBE

Is this value actually used anywhere? I have only found it being used in a condition to detect arm64. If cpu name is used elsewhere, then of course we should either map numerical codes to cpu names or parse the output of hostinfo instead, which gives out cpu name as a string among other info.

P. S. There is no urgency with this PR (unlike the one fixing headers), since it is more of an improvement sort. Let’s discuss options how to do this in a better way.

barracuda156 avatar Jan 28 '24 21:01 barracuda156

The return value of Platform_getCPUBrandString() is currently only used in a single check:

https://github.com/htop-dev/htop/blob/240dd4ab50e1a753905c711f99c57408e18d08a3/darwin/PlatformHelpers.c#L114

What about removing the warning or only show it in debug mode?

cgzones avatar Mar 27 '24 20:03 cgzones

The only usage might even be removed, see #1285.

cgzones avatar Mar 27 '24 20:03 cgzones

@cgzones Thanks. Then I guess we close this for now?

Once there will be a practical case for detecting PowerPC on macOS, this can be reopened and adjusted accordingly.

barracuda156 avatar Mar 27 '24 20:03 barracuda156