On Tue, May 16, 2023 at 10:05:54AM +0300, Dan Carpenter wrote:
> On Mon, May 15, 2023 at 08:59:17PM +0200, Joel Granados wrote:
> > On Mon, May 15, 2023 at 10:34:44AM +0300, Dan Carpenter wrote:
> > > 3756e44223c286 Joel Granados        2023-05-12  504  	if (register_sysctl(tmp_dir_path, t->device_dir) == NULL)
> > > 3756e44223c286 Joel Granados        2023-05-12  505  		/*
> > > 3756e44223c286 Joel Granados        2023-05-12  506  		 * We keep the original behavior of parport where failure to register
> > > 3756e44223c286 Joel Granados        2023-05-12  507  		 * does not return error.
> > > 3756e44223c286 Joel Granados        2023-05-12  508  		 */
> > > 3756e44223c286 Joel Granados        2023-05-12 @509  		goto  exit_free_tmp_dir_path;
> > > 
> > > Smatch wants a error code for this error path.
> > > 
> > > The comments say that we are doing the wrong thing deliberately to
> > > maintain backwards compatibility and avoid breaking stuff.  Fortunately,
> > > (LOL), no one is checking the returns from parport_proc_register() so we
> > > are free to return a correct error code without any worry about breakage.
> > Sigh. Indeed, the only place were this function is called is from
> > parport/share.c. Should we just change the function return value to
> > void?
> 
> I feel like we should return errors and not check for them.
> 
> Hopefully, some day in the future someone will be able to test this code
> and add checks.  (This seems more optimistic than realistic).
looked at this a bit more and saw that all the callers to
parport_announce_port (which is the only function that calls
parport_proc_register) have a concept of an erroneous flow.
In practice it would "just" be a matter of going throuh all these calls
and making parport_announce_port set the appropriate error handle and
return on error.

Hopefully someday, someone will push this :).

> 
> regards,
> dan carpenter
> 

-- 

Joel Granados