On Sat, Jan 1, 2011 at 5:45 PM, roel kluin <roel.kluin@xxxxxxxxx> wrote: > The WRITE_{6,10,12} are defined numbers, so the branch was always taken. > > Signed-off-by: Roel Kluin <roel.kluin@xxxxxxxxx> > --- > drivers/scsi/arcmsr/arcmsr_hba.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > See include/scsi/scsi.h:58-123 for > #define WRITE_6 0x0a > #define WRITE_10 0x2a > #define WRITE_12 0xaa > > diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c > index 17e3df4..a6e5222 100644 > --- a/drivers/scsi/arcmsr/arcmsr_hba.c > +++ b/drivers/scsi/arcmsr/arcmsr_hba.c > @@ -1171,7 +1171,8 @@ static int arcmsr_build_ccb(struct AdapterControlBlock *acb, > arcmsr_cdb->msgPages = arccdbsize/0x100 + (arccdbsize % 0x100 ? 1 : 0); > if ( arccdbsize > 256) > arcmsr_cdb->Flags |= ARCMSR_CDB_FLAG_SGL_BSIZE; > - if (pcmd->cmnd[0]|WRITE_6 || pcmd->cmnd[0]|WRITE_10 || pcmd->cmnd[0]|WRITE_12 ){ > + if (pcmd->cmnd[0] & WRITE_6 || pcmd->cmnd[0] & WRITE_10 || > + pcmd->cmnd[0] & WRITE_12 ){ I'm not sure, if the compiler will optimize this anyway, but would it look better this way: if (pcmd->cmnd[0] & (WRITE_6 | WRITE_10 | WRITE_12)){ And that would open the question what really was meant, as that will be equal to: if (pcmd->cmnd[0] & WRITE_12){ (if you look at the constants, you can skip WRITE_6 and _10 as there bits are already included in WRITE_12) > arcmsr_cdb->Flags |= ARCMSR_CDB_FLAG_WRITE; > } > ccb->arc_cdb_size = arccdbsize; > -- > 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/ > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html