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

[Patrick Mansfield <patmans@us.ibm.com>]

On Wed, May 21, 2003 at 05:11:41PM -0400, Luben Tuikov wrote:

> Mike Anderson wrote:

> > 
> > Why compare with MAX_SN and not just let the value role over and check
> > for false. 
> 
> First, zero is reserved, and I was trying to keep the _next_ sn in
> the ``session'' cmdsn variable, but there's a little _bug_ up there too,
> so here's a corrected version:
> 
> 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 += 2)++  :
> 		shost->serial_number++;

My less than $.02:

As far as the code goes, it does not matter whether we store the current
or next value in shost->serial_number, so we ought to use the simpler code
(Mike's version).  And (x += 2)++ won't compile.

Per naming: "get" is already overloaded and implies a put. We have code
like scsi_get_cmd that allocates and returns a pointer/value, and then
various scsi and other kernel get/put functions that take a pointer and
increment or decrement ref counters. cmdsn is a rather cryptic
abbreviation that my brain can't easily parse.

I suggest scsi_serial_number(host) or scsi_cmdsn(host).

-- Patrick Mansfield