Re: [PATCH 1/1] EDAC: always return an initialized value in knl_show_interleave_mode

[Borislav Petkov]

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.