Re: [PATCH] 2.5.10 IDE 42

[Oliver Xymoron]

On Fri, 26 Apr 2002, Linus Torvalds wrote:

>
>
> On Fri, 26 Apr 2002, Pavel Machek wrote:
> > > +		if (stat & READY_STAT)
> > > +			printk("DriveReady ");
> > > +		if (stat & WRERR_STAT)
> > > +			printk("DeviceFault ");
> > > +		if (stat & SEEK_STAT)
> > > +			printk("SeekComplete ");
> > > +		if (stat & DRQ_STAT)
> > > +			printk("DataRequest ");
> > > +		if (stat & ECC_STAT)
> > > +			printk("CorrectedError ");
> > > +		if (stat & INDEX_STAT)
> > > +			printk("Index ");
> > > +		if (stat & ERR_STAT)
> > > +			printk("Error ");
> >
> > I believe this is actually making it *less* readable.
>
> Somewhat agreed. Also, the above is just not the right way to do
> printouts.
>
> I'd suggest rewriting the whole big mess as something like
>
> 	#define STAT_STR(x,s) \
> 		((stat & x ##_STAT) ? s " " : "")
>
> 	...
>
> 	printf("IDE: %s%s%s%s%s%s..\n"
> 		STAT_STR(READY, "DriveReady"),
> 		STAT_STR(WERR, "DeviceFault"),
> 		...

I'd go even further and suggest that pulling the mapping from a mask or
key to string out into a table and looping through it is preferable for
this sort of thing. You win later if you decide you need the same mapping
elsewhere or if you want to format your messages differently, add bits to
it, etc. Tables are generally easier to inspect for errors than code,
although Linus' version is very nearly a table.

Maintaining tables as code is a pain and is generally only a win for
small or sparse state machines.

-- 
 "Love the dolphins," she advised him. "Write by W.A.S.T.E.."