Messages in this thread | | | Date | Tue, 27 Feb 2018 19:02:31 +0100 | From | Borislav Petkov <> | Subject | Re: [PATCH v2 3/8] efi: Decode IA32/X64 Processor Error Info Structure |
| |
On Tue, Feb 27, 2018 at 05:46:54PM +0000, Ghannam, Yazen wrote: > I think there's value in following the conventions in a subsystem.
"conventions in a subsystem" my ass. That's brainless copy-pasting.
It was added by
f6edea77c8c8 ("ACPI, APEI, CPER: Cleanup CPER memory error output format")
and then replicated everywhere.
It is simply a dumb way of writing:
snprintf(newpfx, sizeof(newpfx), "%s ", pfx);
> I can change this if you give a reason besides "it's dumb".
Two can play that game: you get to keep it if you give a good reason why.
> We do map the spec-defined GUIDs in patch 4 of this set. I don't know if there's > a central place where all vendor-defined GUIDs are listed. I can look into this.
Yes, at least for the most prominent ones.
> And the raw value should still be printed because > 1) It may represent a type that we can't decode. Maybe a type that's not part of > the spec.
If we can't decode it, *then* you dump it:
"Unrecognized type: 0x%llx ..."
> 2) It's good to have the raw value for reference. We do this with MCA_STATUS > where we print the raw value followed by the decoding.
1. No one stares at the raw value if the bits are decoded 2. MCA_STATUS is one register - this error record is huge.
> The structs are all different even though some fields may be the same.
Fair enough. Only if it makes sense.
-- Regards/Gruss, Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --
| |