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: <ZDhXtDLiTtm2iXGW@sai>
Date:   Thu, 13 Apr 2023 21:27:48 +0200
From:   Wolfram Sang <wsa@...nel.org>
To:     Arnd Bergmann <arnd@...nel.org>
Cc:     Jean-Marie Verdun <verdun@....com>,
        Nick Hawkins <nick.hawkins@....com>,
        Joel Stanley <joel@....id.au>, Arnd Bergmann <arnd@...db.de>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] i2c: gxp: fix build failure without CONFIG_I2C_SLAVE

Hi Arnd,

> The gxp_i2c_slave_irq_handler() is hidden in an #ifdef, but the
> caller uses an IS_ENABLED() check:
> 
> drivers/i2c/busses/i2c-gxp.c: In function 'gxp_i2c_irq_handler':
> drivers/i2c/busses/i2c-gxp.c:467:29: error: implicit declaration of function 'gxp_i2c_slave_irq_handler'; did you mean 'gxp_i2c_irq_handler'? [-Werror=implicit-function-declaration]
> 
> It has to consistently use one method or the other to avoid warnings,
> so move to IS_ENABLED() here for readability and build coverage, and
> move the #ifdef in linux/i2c.h to allow building it as dead code.

Can't we have a solution which modifies this driver only (maybe by
defining an empty irq handler for the non-IS_ENABLED part?)? Doesn't
feel good to touch i2c.h only because of this...

> -#if IS_ENABLED(CONFIG_I2C_SLAVE)
>  enum i2c_slave_event {
>  	I2C_SLAVE_READ_REQUESTED,
>  	I2C_SLAVE_WRITE_REQUESTED,
> @@ -396,9 +395,10 @@ enum i2c_slave_event {
>  
>  int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb);
>  int i2c_slave_unregister(struct i2c_client *client);

... especially with moving these two prototypes out of the protected
block. The functions themselves are also protected by the same symbol
via the Makefile. I'd rather get a build error right away than a linker
error later if a driver misses to select I2C_SLAVE. Or do I miss
something?

> -bool i2c_detect_slave_mode(struct device *dev);
>  int i2c_slave_event(struct i2c_client *client,
>  		    enum i2c_slave_event event, u8 *val);
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> +bool i2c_detect_slave_mode(struct device *dev);
>  #else
>  static inline bool i2c_detect_slave_mode(struct device *dev) { return false; }
>  #endif

All the best,

   Wolfram


Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ