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.."- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Tue Apr 30 2002 - 22:00:13 EST