On Thu, 2021-04-01 at 18:33 -0700, Bart Van Assche wrote:
> On 4/1/21 6:09 PM, Laurence Oberman wrote:
> > #define sd_printk(prefix, sdsk, fmt, a...)
> > \
> > - (sdsk)->disk ?
> > \
> > - sdev_prefix_printk(prefix, (sdsk)->device, \
> > + do {
> > \
> > + if (!storage_quiet_discovery)
> > \
> > + (sdsk)->disk ?
> > \
> > + sdev_prefix_printk(prefix, (sdsk)->device, \
> > (sdsk)->disk->disk_name, fmt, ##a) :
> > \
> > - sdev_printk(prefix, (sdsk)->device, fmt, ##a)
> > + sdev_printk(prefix, (sdsk)->device, fmt, ##a);
> > \
> > + } while (0)
>
> The indentation inside the above macro looks odd to me. I guess that
> you
> want to avoid deep indentation? Consider using if (...) break;
> instead
> to reduce the indentation level. Additionally, please change the
> ternary
> operator into an if-condition since the result of the ternary
> operator
> is not used. How about rewriting the above macro as follows?
>
> do {
> if (storage_quiet_discovery)
> break;
> if ((sdk)->disk)
> [ ... ]
> else
> [ ... ]
> } while (0)
>
> Thanks,
>
> Bart.
>
Hi Bart
Yes, Thanks I can try that option for the macro and clean it up.
I will wait a bit for others to review so I can attend to all changes
at once.
Note that the original version was indented like that too and has the
ternary.
See here:
#define sd_printk(prefix, sdsk, fmt,
a...) \
(sdsk)->disk
? \
sdev_prefix_printk(prefix, (sdsk)-
>device, \
(sdsk)->disk->disk_name, fmt, ##a)
: \
sdev_printk(prefix, (sdsk)->device, fmt, ##a)
