Re: [PATCH 2/2] ide-cd: cdrom_decode_status: use return codesinstead of naked numbers

From: Borislav Petkov
Date: Tue Apr 07 2009 - 02:39:47 EST


Hi,

On Mon, Apr 06, 2009 at 11:00:15PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Sunday 05 April 2009, Borislav Petkov wrote:
> > Remove unused SECTOR_SIZE while at it.
> >
> > There should be no functional change resulting from this patch.
> >
> > Signed-off-by: Borislav Petkov <petkovbb@xxxxxxxxx>
>
> [...]
>
> > @@ -431,7 +425,7 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
> > /* if we got a CHECK_CONDITION status, queue a request sense command */
> > if (stat & ATA_ERR)
> > cdrom_queue_request_sense(drive, NULL, NULL);
> > - return 1;
> > + return REQ_RECOVER;
> >
> > end_request:
> > if (stat & ATA_ERR) {
> > @@ -445,9 +439,9 @@ end_request:
> > hwif->rq = NULL;
> >
> > cdrom_queue_request_sense(drive, rq->sense, rq);
> > - return 1;
> > - } else
> > - return 2;
> > + return REQ_RECOVER;
> > + }
> > + return REQ_FAIL;
> > }
>
> Could it be that cdrom_newpc_intr() chunk got lost somewhere along the way,
> IIRC it was there?

That I dropped :), my bad.

> > +/* internal decode_status codes */
> > +#define REQ_CONT 0
> > +#define REQ_RECOVER 1
> > +#define REQ_FAIL 2
>
> Did you notice my comments about REQ_* in previous mail?

Yep, about those - is there a valid reason for calling them IDE_RQ_*?
I know REQ_* is too generic but since they're private to the driver it
really is the only proper naming you _can_ have without adding too much
information to the name. FWIW, SCSI has even more generic names for them
- SUCCESS, FAILED, etc. And the IDE_* prefix is only then called for
when they're going to be visible/used by some other IDE parts. So IMHO
REQ_* or RQ_* is actually better in this case.

--
Regards/Gruss,
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/