Messages in this thread | | | Date | Sun, 22 Jan 2017 15:41:44 +0100 | From | Borislav Petkov <> | Subject | Re: [PATCH 1/1] EDAC: always return an initialized value in knl_show_interleave_mode |
| |
On Sun, Jan 22, 2017 at 02:51:15PM +0100, Nicolas Iooss wrote: > When building drivers/edac/sb_edac.c with compiler warning flags which > aim to detect the use of uninitialized values at compile time, the > compiler reports that knl_show_interleave_mode() may return an > uninitialized value. This function indeed uses a switch statement to set > a local variable ("s"), which is not set to anything in the default > statement. Anyway this would be never reached as > knl_interleave_mode(reg) always returns a value between 0 and 3, but the > compiler has no way of knowing this. > > Silent the compiler warning by initializing variable "s" too in the > default case. While at it, make knl_show_interleave_mode() and > show_interleave_mode() return const char* values as the returned > pointers refer to read-only memory.
No, this is trying to make pretty an already ugly pile of crap.
That ->show_interleave_mode() is called only once in a debug printk. Big deal. But it is a function pointer which points to the same function except for KNL.
Then, the default implementation returns bit slices: "8:6" : "[8:6]XOR[18:16]" but the KNL one says "use address bits" like this is the SDM. Yeah, right. I should've caught that when the KNL pile was added.
So here's what I'd like you to do, instead:
Kill that ->show_interleave_mode() thing and use the default show_interleave_mode() at the only call site. You can pass in a second bool argument is_knl which is "pvt->info.type == KNIGHTS_LANDING".
And then add the KNL functionality from knl_show_interleave_mode() to the default show_interleave_mode().
And get rid of that default: case - it won't be reached anyway.
You can even define a string array:
struct pair intlv_mode[] = { "[8:6]", "[10:8]", ...; };
and then do:
if (!is_knl && !interleave_mode(reg)) return "[8:6]XOR[18:16]"; else return intlv_mode[knl_interleave_mode()];
and be done with it.
Thanks.
-- Regards/Gruss, Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
| |