Re: [PATCH v2] tty: vt: keyboard: initialize "kbs" so that kfree(kbs) runs fine even if kbs is not kmalloced.

[Ajay Garg]

First of all, many thanks to Pavel for all the help and guidance,
nature bless you.
I will make it a point to keep all maintainers in loop, for all my
future patches.

>
> Everybody who is developing for kernel may check this easily, no need
> to have this in the commit message.
>
> As I told you, NAK.
> This is no value in this patch according to the commit message.
>
> If you have a compiler warning you need to provide the command line
> for `make` that makes that warning appear.

"make" as such runs fine, without any blocker (on my x86_64 machine).

The "kbs not initialized" is seen, when the smatch static-analyzer is run.
Thereafter, the patch was floated, to make the method
"vt_do_kdgkb_ioctl" crash-proof by handling kbs properly, without
depending upon external factors on whether a switch-case is hit or
not.

> In that case the better
> solution would be to add default case because some compilers can make
> (wrong) assumptions based on the absence of the default case.
>
> Something like
>
> default:
>    kbs = NULL;
>    break;
>
> at the end of the switch.
>
> But again, your current commit message does not sell.

Hmm, am not sure what to make of this.

I guess, we could follow one of the following approaches :

i)
Leave things as it is, as there is no blocker in the make-compilation/runtime.

ii)
Put the "default: kbs = NULL; break;" case, as suggested, to ensure
"vt_do_kdgkb_ioctl" does its work fine flawlessly, without depending
upon external clients.


Since there is no blocker in functionality, I am ok with whatever
consensus is reached by the maintainers.
If we wish to go with ii), please let me know, I will float next
patch-version with the ii) change, along with a better commit-message.

As of now, since Andy has voted for a NAK, I would not pursue this
further as of now :)


Thanks and Regards,
Ajay