drivers/cdrom/cdrom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Signed-off-by: Joey Pabalinas <joeypabalinas@gmail.com>
---
drivers/cdrom/cdrom.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index 31ba1f8c1f7865dc99..462ee74621da6f32da 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -408,11 +408,11 @@ static int cdrom_get_disc_info(struct cdrom_device_info *cdi,
* hack to have the capability flags defined const, while we can still
* change it here without gcc complaining at every line.
*/
#define ENSURE(cdo, call, bits) \
do { \
- if (cdo->call == NULL) \
+ if ((cdo)->(call) == NULL) \
WARN_ON_ONCE((cdo)->capability & (bits)); \
} while (0)
/*
* the first prototypes used 0x2c as the page code for the mrw mode page,
--
Cheers,
Joey Pabalinas
On Fri, Sep 05, 2025 at 07:59:40AM -1000, Joey Pabalinas wrote:
> Signed-off-by: Joey Pabalinas <joeypabalinas@gmail.com>
> ---
> drivers/cdrom/cdrom.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> index 31ba1f8c1f7865dc99..462ee74621da6f32da 100644
> --- a/drivers/cdrom/cdrom.c
> +++ b/drivers/cdrom/cdrom.c
> @@ -408,11 +408,11 @@ static int cdrom_get_disc_info(struct cdrom_device_info *cdi,
> * hack to have the capability flags defined const, while we can still
> * change it here without gcc complaining at every line.
> */
> #define ENSURE(cdo, call, bits) \
> do { \
> - if (cdo->call == NULL) \
> + if ((cdo)->(call) == NULL) \
You do realize that the right-hand argument of . and -> is *not* an
expression, right? How would anything other than identifier work, anyway?
Note, BTW, that such identifier would not work as a standalone expression -
its meaning depends upon the structure of union you are dealing with...
General advise, from painful personal experience: before posting a patch,
make sure that you have
* got enough caffeine in your bloodstream
* looked through the patch
* tried to compile the results
The order of the last two may vary, but the first one really needs to go
before anything else.
On Sat, Sep 06, 2025 at 07:28:19AM +0100, Al Viro wrote:
> On Fri, Sep 05, 2025 at 07:59:40AM -1000, Joey Pabalinas wrote:
> > Signed-off-by: Joey Pabalinas <joeypabalinas@gmail.com>
> > ---
> > drivers/cdrom/cdrom.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> > index 31ba1f8c1f7865dc99..462ee74621da6f32da 100644
> > --- a/drivers/cdrom/cdrom.c
> > +++ b/drivers/cdrom/cdrom.c
> > @@ -408,11 +408,11 @@ static int cdrom_get_disc_info(struct cdrom_device_info *cdi,
> > * hack to have the capability flags defined const, while we can still
> > * change it here without gcc complaining at every line.
> > */
> > #define ENSURE(cdo, call, bits) \
> > do { \
> > - if (cdo->call == NULL) \
> > + if ((cdo)->(call) == NULL) \
>
> You do realize that the right-hand argument of . and -> is *not* an
> expression, right? How would anything other than identifier work, anyway?
> Note, BTW, that such identifier would not work as a standalone expression -
> its meaning depends upon the structure of union you are dealing with...
>
> General advise, from painful personal experience: before posting a patch,
> make sure that you have
> * got enough caffeine in your bloodstream
> * looked through the patch
> * tried to compile the results
> The order of the last two may vary, but the first one really needs to go
> before anything else.
Hi Joey,
Thank you for sending the patch. I must add however (in addition to Al's
advice) that there doesn't seem to be a proper commit description
either - just a subject line and a Signed-off-by tag. All commits should
individually contain a description too. I appreciate the effort though,
and I encourage you to make further submissions should you choose to do
so. All the best.
Regards,
Phil
Uniform CDROM Maintainer
Hi Joey,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.17-rc4 next-20250905]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Joey-Pabalinas/cdrom-add-parentheses-around-macro-arguments/20250906-020047
base: linus/master
patch link: https://lore.kernel.org/r/13378f5c9cafc29425b6e420cad8b513f4a9f1e1.1757095005.git.joeypabalinas%40gmail.com
patch subject: [PATCH] cdrom: add parentheses around macro arguments
config: sh-randconfig-r071-20250906 (https://download.01.org/0day-ci/archive/20250906/202509061338.yRYn1te8-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 10.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250906/202509061338.yRYn1te8-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509061338.yRYn1te8-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/cdrom/cdrom.c: In function 'register_cdrom':
>> drivers/cdrom/cdrom.c:413:13: error: expected identifier before '(' token
413 | if ((cdo)->(call) == NULL) \
| ^
drivers/cdrom/cdrom.c:604:2: note: in expansion of macro 'ENSURE'
604 | ENSURE(cdo, drive_status, CDC_DRIVE_STATUS);
| ^~~~~~
>> drivers/cdrom/cdrom.c:413:13: error: expected identifier before '(' token
413 | if ((cdo)->(call) == NULL) \
| ^
drivers/cdrom/cdrom.c:607:2: note: in expansion of macro 'ENSURE'
607 | ENSURE(cdo, tray_move, CDC_CLOSE_TRAY | CDC_OPEN_TRAY);
| ^~~~~~
>> drivers/cdrom/cdrom.c:413:13: error: expected identifier before '(' token
413 | if ((cdo)->(call) == NULL) \
| ^
drivers/cdrom/cdrom.c:608:2: note: in expansion of macro 'ENSURE'
608 | ENSURE(cdo, lock_door, CDC_LOCK);
| ^~~~~~
>> drivers/cdrom/cdrom.c:413:13: error: expected identifier before '(' token
413 | if ((cdo)->(call) == NULL) \
| ^
drivers/cdrom/cdrom.c:609:2: note: in expansion of macro 'ENSURE'
609 | ENSURE(cdo, select_speed, CDC_SELECT_SPEED);
| ^~~~~~
>> drivers/cdrom/cdrom.c:413:13: error: expected identifier before '(' token
413 | if ((cdo)->(call) == NULL) \
| ^
drivers/cdrom/cdrom.c:610:2: note: in expansion of macro 'ENSURE'
610 | ENSURE(cdo, get_last_session, CDC_MULTI_SESSION);
| ^~~~~~
>> drivers/cdrom/cdrom.c:413:13: error: expected identifier before '(' token
413 | if ((cdo)->(call) == NULL) \
| ^
drivers/cdrom/cdrom.c:611:2: note: in expansion of macro 'ENSURE'
611 | ENSURE(cdo, get_mcn, CDC_MCN);
| ^~~~~~
>> drivers/cdrom/cdrom.c:413:13: error: expected identifier before '(' token
413 | if ((cdo)->(call) == NULL) \
| ^
drivers/cdrom/cdrom.c:612:2: note: in expansion of macro 'ENSURE'
612 | ENSURE(cdo, reset, CDC_RESET);
| ^~~~~~
>> drivers/cdrom/cdrom.c:413:13: error: expected identifier before '(' token
413 | if ((cdo)->(call) == NULL) \
| ^
drivers/cdrom/cdrom.c:613:2: note: in expansion of macro 'ENSURE'
613 | ENSURE(cdo, generic_packet, CDC_GENERIC_PACKET);
| ^~~~~~
vim +413 drivers/cdrom/cdrom.c
405
406 /* This macro makes sure we don't have to check on cdrom_device_ops
407 * existence in the run-time routines below. Change_capability is a
408 * hack to have the capability flags defined const, while we can still
409 * change it here without gcc complaining at every line.
410 */
411 #define ENSURE(cdo, call, bits) \
412 do { \
> 413 if ((cdo)->(call) == NULL) \
414 WARN_ON_ONCE((cdo)->capability & (bits)); \
415 } while (0)
416
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2025 Red Hat, Inc.