[PATCH v2 1/1] input: avoid NULL dereference

[Tom Rini]

On Tue, Oct 03, 2023 at 01:37:14AM +0200, Rasmus Villemoes wrote:
> On 03/10/2023 00.46, Tom Rini wrote:
> > On Tue, Oct 03, 2023 at 12:27:25AM +0200, Heinrich Schuchardt wrote:
> >>  
> >>  	error = stdio_register(dev);
> >> -#if !defined(CONFIG_SPL_BUILD) || CONFIG_IS_ENABLED(ENV_SUPPORT)
> >> -	/* check if this is the standard input device */
> >> -	if (!error && strcmp(env_get("stdin"), dev->name) == 0) {
> >> -		/* reassign the console */
> >> -		if (OVERWRITE_CONSOLE ||
> >> -				console_assign(stdin, dev->name))
> >> -			return -1;
> >> +	if ((IS_ENABLED(SPL_BUILD) || CONFIG_IS_ENABLED(ENV_SUPPORT)) &&
> >> +	    !error) {
> >> +		const char *cstdin;
> >> +
> >> +		/* check if this is the standard input device */
> >> +		cstdin = env_get("stdin");
> >> +		if (cstdin && !strcmp(cstdin, dev->name)) {
> >> +			/* reassign the console */
> >> +			if (OVERWRITE_CONSOLE ||
> >> +			    console_assign(stdin, dev->name))
> >> +				return -1;
> >> +		}
> >>  	}
> >> -#else
> >> -	error = error;
> >> -#endif
> >>  
> >>  	return 0;
> >>  }
> > 
> > This is an example I think of where #if is more readable.
> > 
> 
> I kind of agree, but at the same time, this could just be rewritten to
> avoid that extra indentation. Turn the condition around and make it an
> early return, then the rest of the function need not be indented.

Yes, both the indentation and having to use "CONFIG_SPL_BUILD" to start
with (which is a special case!).

> I also think the conversion above is buggy (loses a ! on the SPL part),
> but also, the condition is needlessly convoluted to begin with.
> !defined(CONFIG_SPL_BUILD) implies ENV_SUPPORT, so the condition is
> equivalent to just CONFIG_IS_ENABLED(ENV_SUPPORT). So I'd add a
> 
>   if (!CONFIG_IS_ENABLED(ENV_SUPPORT)
>     return 0;

Changes like that would make things overall better, agreed.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20231002/e9d43c9e/attachment.sig>