Re: [PATCH 10/12] ide-floppy: remove atomic test_*bit macros

[Borislav Petkov]

On Mon, Jan 14, 2008 at 10:50:58PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Sunday 13 January 2008, Borislav Petkov wrote:
> > ..and replace them with flag enums.
> > 
> > Signed-off-by: Borislav Petkov <bbpetkov@xxxxxxxx>
> > ---
> >  drivers/ide/ide-floppy.c |  132 +++++++++++++++++++++++++--------------------
> >  1 files changed, 73 insertions(+), 59 deletions(-)
> 
> [...]
> 
> > @@ -506,14 +516,14 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)
> >  
> >  	debug_log("Reached %s interrupt handler\n", __FUNCTION__);
> >  
> > -	if (test_bit(PC_DMA_IN_PROGRESS, &pc->flags)) {
> > +	if (PC_FLAG_DMA_IN_PROGRESS & pc->flags) {
> 
> the usual kernel convention is to put flag last, i.e.
> 
> pc->flags & PC_FLAG_DMA_IN_PROGRESS
> 
> [...]
> 
> > @@ -570,7 +581,7 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)
> >  		printk(KERN_ERR "ide-floppy: CoD != 0 in %s\n", __FUNCTION__);
> >  		return ide_do_reset(drive);
> >  	}
> > -	if (((ireason & IO) == IO) == test_bit(PC_WRITING, &pc->flags)) {
> > +	if (((ireason & IO) == IO) == (PC_FLAG_WRITING  & pc->flags)) {
> 
> - test_bit() returns 1 or 0 (=> boolean)
> - (pc->flags & PC_FLAG_WRITING) is 0x10 or 0
> 
> so the above comparison will fail
> 
> > @@ -607,7 +618,7 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive)
> >  		xferfunc(drive, pc->current_position, bcount);
> >  	else
> >  		ide_floppy_io_buffers(drive, pc, bcount,
> > -				test_bit(PC_WRITING, &pc->flags));
> > +				(PC_FLAG_WRITING & pc->flags));
> 
> ditto, this may actually work but '(pc->flags & PC_FLAG_WRITING) ? 1 : 0'
> would be much safer from maintainability POV
Hi Bart,

i must've been sleeping while i've been doing this :) Will redo them tonight and
resend.

Thanks.
-- 
Regards/Gruß,
    Boris.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/