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.
