Re: [PATCH] cdrom: add parentheses around macro arguments

[Phillip Potter <phil@philpotter.co.uk>]

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