Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx> wrote: > On Wednesday 13 August 2008, Elias Oltmanns wrote: >> Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx> wrote: > >> > Hi, >> > >> > On Monday 11 August 2008, Elias Oltmanns wrote: [...] >> >> + rq->cmd_len = DEVSET_CDB_LEN; >> >> + ds = (ide_devset_cdb_t *)rq->cmd; >> >> + ds->opcode = REQ_DEVSET_EXEC; >> >> + ds->arg = arg; >> > >> > How's about just doing: >> > >> > rq->cmd[0] = REQ_DEVSET_EXEC; >> > (int *)rq->cmd[1] = arg; >> >> CC [M] drivers/ide/ide-io.o >> drivers/ide/ide-io.c: In function 'ide_devset_execute': >> drivers/ide/ide-io.c:748: warning: cast to pointer from integer of different size >> drivers/ide/ide-io.c:748: error: lvalue required as left operand of assignment > > Heh, I must have been falling asleep already when writing this, > it should have been: > > *(int *)&rq->cmd[1] = arg; Well, this I can make sense of. > >> Personally, I'd feel more comfortable with casting rq->cmd to a >> dedicated struct, especially since I'm not exactly a wizard when it >> comes to casting. Naturally, if you prefer something else (and I get it >> to work), I'll happily accept that too. > > What I find ugly about this dedicated struct is that it overlaps with > rq->cmd[0] and in all other places we just use rq->cmd[0] directly > (+ it is very easy for people unfamiliar with the code to overlook it). Yes, I see. > > Then if 'opcode' gets removed all that is left is 'arg' so the struct > no loger makes much sense. > > I fixed it locally (interdiff attached), I hope you're fine with it. Certainly. Regards, Elias -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
