linux-kernel - Re: [PATCH v2 1/1] i2c: parport: Switch to use module_parport

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210721102146.66a486bc@endymion>
Date:   Wed, 21 Jul 2021 10:21:46 +0200
From:   Jean Delvare <jdelvare@...e.de>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/1] i2c: parport: Switch to use
 module_parport_driver()

Hi Andy,

On Mon, 12 Jul 2021 17:11:19 +0300, Andy Shevchenko wrote:
> Switch to use module_parport_driver() to reduce boilerplate code.

This has the downside of moving the sanity check of the type parameter
to run time, instead of driver load time. In particular this means that
loading the i2c-parport driver without specifying the type will no
longer fail. The driver will load successfully, but won't do anything.

While I prefer user errors to be reported as soon as possible, I don't
really mind here, as parallel port drivers are not something worth
debating over at this point in time. As a matter of fact, I can't
possibly test this change as I no longer have a parallel port on any of
my systems.

So if that's the direction we want to take then so be it.

Reviewed-by: Jean Delvare <jdelvare@...e.de>

> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> ---
> v2: fixed compilation error (Jean, LKP)
>  drivers/i2c/busses/i2c-parport.c | 36 ++++++++++----------------------
>  1 file changed, 11 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-parport.c b/drivers/i2c/busses/i2c-parport.c
> index a535889acca6..231145c48728 100644
> --- a/drivers/i2c/busses/i2c-parport.c
> +++ b/drivers/i2c/busses/i2c-parport.c
> @@ -267,6 +267,16 @@ static void i2c_parport_attach(struct parport *port)
>  	int i;
>  	struct pardev_cb i2c_parport_cb;
>  
> +	if (type < 0) {
> +		pr_warn("adapter type unspecified\n");
> +		return;
> +	}
> +
> +	if (type >= ARRAY_SIZE(adapter_parm)) {
> +		pr_warn("invalid type (%d)\n", type);
> +		return;
> +	}
> +
>  	for (i = 0; i < MAX_DEVICE; i++) {
>  		if (parport[i] == -1)
>  			continue;
> @@ -392,32 +402,8 @@ static struct parport_driver i2c_parport_driver = {
>  	.detach = i2c_parport_detach,
>  	.devmodel = true,
>  };
> -
> -/* ----- Module loading, unloading and information ------------------------ */
> -
> -static int __init i2c_parport_init(void)
> -{
> -	if (type < 0) {
> -		pr_warn("adapter type unspecified\n");
> -		return -ENODEV;
> -	}
> -
> -	if (type >= ARRAY_SIZE(adapter_parm)) {
> -		pr_warn("invalid type (%d)\n", type);
> -		return -ENODEV;
> -	}
> -
> -	return parport_register_driver(&i2c_parport_driver);
> -}
> -
> -static void __exit i2c_parport_exit(void)
> -{
> -	parport_unregister_driver(&i2c_parport_driver);
> -}
> +module_parport_driver(i2c_parport_driver);
>  
>  MODULE_AUTHOR("Jean Delvare <jdelvare@...e.de>");
>  MODULE_DESCRIPTION("I2C bus over parallel port");
>  MODULE_LICENSE("GPL");
> -
> -module_init(i2c_parport_init);
> -module_exit(i2c_parport_exit);


-- 
Jean Delvare
SUSE L3 Support