Re: Patch: change the serial_number for error-handler commands

[Mike Anderson <andmike@us.ibm.com>]

Luben Tuikov [tluben@rogers.com] wrote:
> >+static inline unsigned long scsi_get_next_serial(struct Scsi_Host *shost)
> >+{
> >+        return (++shost->serial_number) ? shost->serial_number : 1;
> >+}
> >+
> 

There is a bug in the above line it should be 
return (++shost->serial_number) ? shost->serial_number : ++shost->serial_number;

> scsi_get_next_serial()???  Do _you_ Mike like this name?
> 
No.

> scsi_get_cmdsn()  will be a lot more appropriate.  It doesn't
> reveal implementation (``next''), and tells that it's a serial
> number for a command, ``sn'' stands for serial number pretty
> universally in our society.
> 

This name is ok with me.

> How about something like this:
> 
> static inline unsigned long scsi_get_cmdsn(struct Scsi_Host, *shost)
> {
> 	static const typeof(shost->serial_number) MAX_SN = 
> 		~((typeof(shost->serial_number) 0);
> 	return  shost->serial_number++ == MAX_SN ?
> 		++shost->serial_number : shost->serial_number;
> }
> 

Why compare with MAX_SN and not just let the value role over and check
for false. 

> BTW, are you relying on memset(..., 0, sizeof(struct Scsi_Host),
> to set the serial number to 0?

Yes I was.

-andmike
--
Michael Anderson
andmike@us.ibm.com