liske / needrestart

Restart daemons after library updates.
GNU General Public License v2.0
420 stars 67 forks source link

Improvement to CPU Vendor tests #284

Closed fritz-fritz closed 1 month ago

fritz-fritz commented 10 months ago

In response to #274, this should prevent Intel uCode from being attempted on AMD. It has not been tested on Intel CPUs however and should be before merging.

The existing check from the AMD.pm was implemented for the Intel.pm in the same style. Both were updated to use a case-insensitive regex.

stbuehler commented 9 months ago

As I said in #274 I think GenuineIntel would probably the more appropriate check for intel, and I don't see why you'd want to change AuthenticAMD to /amd/i unless you can show the previous check is a problem.

(I assume that for both intel and amd we always get those strings for "official" cpus. No idea whether there are CPUs out there that claim to be somewhat compatible that would match /intel|amd/i, but if there are, I'd be very surprised if they work with the microcode handling here.)

Apart from that the changes look good to me - works on both intel and amd for me.

fritz-fritz commented 1 month ago

Sorry for letting this sit, I initially used a loose regex on Intel as I wasn't sure if I could rely on GenuineIntel to be standard and have no Intel to test on. So then I adapted the AMD check to match just to keep the code consistent.

I went ahead and pushed a new commit changing both to hardcoded checks on AuthenticAMD and GenuineIntel. I am also willing to rebase to squash the commit history if you'd like. Otherwise I think this is good to merge :rocket:

liske commented 1 month ago

Thanks!