linux-kernel - Re: [PATCH] HID-PICOLCD: Dummy hid-picolcd functions should return error code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20160622093337.68d1bb9a@pluto.restena.lu>
Date:	Wed, 22 Jun 2016 09:33:37 +0200
From:	Bruno Prémont <bonbons@...ux-vserver.org>
To:	Arvind Yadav <arvind.yadav.cs@...il.com>
Cc:	jikos@...nel.org, benjamin.tissoires@...hat.com,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] HID-PICOLCD: Dummy hid-picolcd functions should return
 error code

Hi Arvind,

I can only NACK this patch as it would cause the driver to refuse
probe()ing if any of the features are disabled in kernel configuration.

Have a look at the callers in hid-picolcd_code.c, if any of those
functions does return an error the probe function aborts with an error.


Your commit message talks about allowing compilation if features
are disabled, so which compilation errors do you get that changing
those return codes "fix"?
If you don't have such errors, I don't get your motivations for the
change out of your commit message.

Regards,
Bruno


On Wed, 22 Jun 2016 00:08:40 +0530 Arvind Yadav wrote:
> - inline picolcd_fb_reset and picolcd_init_framebuffer stub simply
>   allows compilation on systems with CONFIG_HID_PICOLCD_FB disabled.
> - inline picolcd_init_backlight and picolcd_resume_backlight stub
>   simply allows compilation on systems with CONFIG_HID_PICOLCD_BACKLIGHT
>   disabled.
> - inline picolcd_init_lcd and picolcd_resume_lcd stub simply allows
>   compilation on systems with CONFIG_HID_PICOLCD_LCD disabled.
> - inline picolcd_init_leds stub simply allows compilation on systems
>   with CONFIG_HID_PICOLCD_LEDS disabled.
> - inline picolcd_init_cir stub simply allows compilation on systems
>   with CONFIG_HID_PICOLCD_CIR disabled.
> 
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@...il.com>
> ---
>  drivers/hid/hid-picolcd.h | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/hid/hid-picolcd.h b/drivers/hid/hid-picolcd.h
> index e56d847..7e6c10a 100644
> --- a/drivers/hid/hid-picolcd.h
> +++ b/drivers/hid/hid-picolcd.h
> @@ -17,6 +17,8 @@
>   *   along with this software. If not see <http://www.gnu.org/licenses/>.  *
>   ***************************************************************************/
>  
> +#include <linux/errno.h>
> +
>  #define PICOLCD_NAME "PicoLCD (graphic)"
>  
>  /* Report numbers */
> @@ -192,11 +194,11 @@ void picolcd_fb_refresh(struct picolcd_data *data);
>  #else
>  static inline int picolcd_fb_reset(struct picolcd_data *data, int clear)
>  {
> -	return 0;
> +	return -ENODEV;
>  }
>  static inline int picolcd_init_framebuffer(struct picolcd_data *data)
>  {
> -	return 0;
> +	return -ENOMEM;
>  }
>  static inline void picolcd_exit_framebuffer(struct picolcd_data *data)
>  {
> @@ -221,14 +223,14 @@ void picolcd_suspend_backlight(struct picolcd_data *data);
>  static inline int picolcd_init_backlight(struct picolcd_data *data,
>  		struct hid_report *report)
>  {
> -	return 0;
> +	return -ENODEV;
>  }
>  static inline void picolcd_exit_backlight(struct picolcd_data *data)
>  {
>  }
>  static inline int picolcd_resume_backlight(struct picolcd_data *data)
>  {
> -	return 0;
> +	return -ENODEV;
>  }
>  static inline void picolcd_suspend_backlight(struct picolcd_data *data)
>  {
> @@ -248,14 +250,14 @@ int picolcd_resume_lcd(struct picolcd_data *data);
>  static inline int picolcd_init_lcd(struct picolcd_data *data,
>  		struct hid_report *report)
>  {
> -	return 0;
> +	return -ENODEV;
>  }
>  static inline void picolcd_exit_lcd(struct picolcd_data *data)
>  {
>  }
>  static inline int picolcd_resume_lcd(struct picolcd_data *data)
>  {
> -	return 0;
> +	return -ENODEV;
>  }
>  #endif /* CONFIG_HID_PICOLCD_LCD */
>  
> @@ -271,7 +273,7 @@ void picolcd_leds_set(struct picolcd_data *data);
>  static inline int picolcd_init_leds(struct picolcd_data *data,
>  		struct hid_report *report)
>  {
> -	return 0;
> +	return -ENODEV;
>  }
>  static inline void picolcd_exit_leds(struct picolcd_data *data)
>  {
> @@ -297,7 +299,7 @@ static inline int picolcd_raw_cir(struct picolcd_data *data,
>  }
>  static inline int picolcd_init_cir(struct picolcd_data *data, struct hid_report *report)
>  {
> -	return 0;
> +	return -ENOMEM;
>  }
>  static inline void picolcd_exit_cir(struct picolcd_data *data)
>  {