[janitor] use kernel min/max (2/2)

[janitor] use kernel min/max (2/2)

[Randy.Dunlap]

Hi,
Please apply to 2.6.current.

--
~Randy


From: Michael Veeck <michael.veeck@gmx.net>


Patch (against 2.6.3) removes unnecessary min/max macros and changes
calls to use kernel.h macros instead.

Best Regards
Michael

---

 linux-264-rc2-kj1-rddunlap/drivers/ide/ide-io.c        |    2 +-
 linux-264-rc2-kj1-rddunlap/drivers/ide/ide-proc.c      |    4 ++--
 linux-264-rc2-kj1-rddunlap/drivers/ide/legacy/qd65xx.c |    2 +-
 linux-264-rc2-kj1-rddunlap/drivers/ide/pci/cmd64x.c    |    2 +-
 linux-264-rc2-kj1-rddunlap/drivers/ide/pci/via82cxxx.c |    2 +-
 linux-264-rc2-kj1-rddunlap/drivers/scsi/ide-scsi.c     |   10 +++++-----
 linux-264-rc2-kj1-rddunlap/include/linux/ide.h         |    2 --
 7 files changed, 11 insertions(+), 13 deletions(-)

diff -puN drivers/ide/ide-io.c~drivers_ide_minmax2 drivers/ide/ide-io.c
--- linux-264-rc2-kj1/drivers/ide/ide-io.c~drivers_ide_minmax2	2004-03-08 16:49:15.000000000 -0800
+++ linux-264-rc2-kj1-rddunlap/drivers/ide/ide-io.c	2004-03-08 16:49:15.000000000 -0800
@@ -741,7 +741,7 @@ repeat:	
 				 && 0 < (signed long)(WAKEUP(drive) - (jiffies - best->service_time))
 				 && 0 < (signed long)((jiffies + t) - WAKEUP(drive)))
 				{
-					ide_stall_queue(best, IDE_MIN(t, 10 * WAIT_MIN_SLEEP));
+					ide_stall_queue(best, min_t(long, t, 10 * WAIT_MIN_SLEEP));
 					goto repeat;
 				}
 			} while ((drive = drive->next) != best);
diff -puN drivers/ide/ide-proc.c~drivers_ide_minmax2 drivers/ide/ide-proc.c
--- linux-264-rc2-kj1/drivers/ide/ide-proc.c~drivers_ide_minmax2	2004-03-08 16:49:15.000000000 -0800
+++ linux-264-rc2-kj1-rddunlap/drivers/ide/ide-proc.c	2004-03-08 16:49:15.000000000 -0800
@@ -508,8 +508,8 @@ int proc_ide_write_settings
 			}
 			if (*p != ':')
 				goto parse_error;
-			len = IDE_MIN(p - start, MAX_LEN);
-			strncpy(name, start, IDE_MIN(len, MAX_LEN));
+			len = min(p - start, MAX_LEN);
+			strncpy(name, start, min(len, MAX_LEN));
 			name[len] = 0;
 
 			if (n > 0) {
diff -puN drivers/ide/legacy/qd65xx.c~drivers_ide_minmax2 drivers/ide/legacy/qd65xx.c
--- linux-264-rc2-kj1/drivers/ide/legacy/qd65xx.c~drivers_ide_minmax2	2004-03-08 16:49:15.000000000 -0800
+++ linux-264-rc2-kj1-rddunlap/drivers/ide/legacy/qd65xx.c	2004-03-08 16:49:15.000000000 -0800
@@ -262,7 +262,7 @@ static void qd6580_tune_drive (ide_drive
 
 	if (drive->id && !qd_find_disk_type(drive, &active_time, &recovery_time)) {
 		pio = ide_get_best_pio_mode(drive, pio, 255, &d);
-		pio = IDE_MIN(pio,4);
+		pio = min_t(u8, pio,4);
 
 		switch (pio) {
 			case 0: break;
diff -puN drivers/ide/pci/cmd64x.c~drivers_ide_minmax2 drivers/ide/pci/cmd64x.c
--- linux-264-rc2-kj1/drivers/ide/pci/cmd64x.c~drivers_ide_minmax2	2004-03-08 16:49:15.000000000 -0800
+++ linux-264-rc2-kj1-rddunlap/drivers/ide/pci/cmd64x.c	2004-03-08 16:49:15.000000000 -0800
@@ -200,7 +200,7 @@ static void program_drive_counts (ide_dr
 	 */
 	if (channel) {
 		drive->drive_data = setup_count;
-		setup_count = IDE_MAX(drives[0].drive_data,
+		setup_count = max(drives[0].drive_data,
 					drives[1].drive_data);
 		cmdprintk("Secondary interface, setup_count = %d\n",
 					setup_count);
diff -puN drivers/ide/pci/via82cxxx.c~drivers_ide_minmax2 drivers/ide/pci/via82cxxx.c
--- linux-264-rc2-kj1/drivers/ide/pci/via82cxxx.c~drivers_ide_minmax2	2004-03-08 16:49:15.000000000 -0800
+++ linux-264-rc2-kj1-rddunlap/drivers/ide/pci/via82cxxx.c	2004-03-08 16:49:15.000000000 -0800
@@ -375,7 +375,7 @@ static void via82cxxx_tune_drive(ide_dri
 		return;
 	}
 
-	via_set_drive(drive, XFER_PIO_0 + MIN(pio, 5));
+	via_set_drive(drive, XFER_PIO_0 + min_t(u8, pio, 5));
 }
 
 /**
diff -puN drivers/scsi/ide-scsi.c~drivers_ide_minmax2 drivers/scsi/ide-scsi.c
--- linux-264-rc2-kj1/drivers/scsi/ide-scsi.c~drivers_ide_minmax2	2004-03-08 16:49:15.000000000 -0800
+++ linux-264-rc2-kj1-rddunlap/drivers/scsi/ide-scsi.c	2004-03-08 16:49:15.000000000 -0800
@@ -145,7 +145,7 @@ static void idescsi_input_buffers (ide_d
 			idescsi_discard_data (drive, bcount);
 			return;
 		}
-		count = IDE_MIN (pc->sg->length - pc->b_count, bcount);
+		count = min(pc->sg->length - pc->b_count, bcount);
 		buf = page_address(pc->sg->page) + pc->sg->offset;
 		atapi_input_bytes (drive, buf + pc->b_count, count);
 		bcount -= count; pc->b_count += count;
@@ -167,7 +167,7 @@ static void idescsi_output_buffers (ide_
 			idescsi_output_zeros (drive, bcount);
 			return;
 		}
-		count = IDE_MIN (pc->sg->length - pc->b_count, bcount);
+		count = min(pc->sg->length - pc->b_count, bcount);
 		buf = page_address(pc->sg->page) + pc->sg->offset;
 		atapi_output_bytes (drive, buf + pc->b_count, count);
 		bcount -= count; pc->b_count += count;
@@ -354,7 +354,7 @@ static int idescsi_end_request (ide_driv
 			if (!test_bit(PC_WRITING, &pc->flags) && pc->actually_transferred && pc->actually_transferred <= 1024 && pc->buffer) {
 				printk(", rst = ");
 				scsi_buf = pc->scsi_cmd->request_buffer;
-				hexdump(scsi_buf, IDE_MIN(16, pc->scsi_cmd->request_bufflen));
+				hexdump(scsi_buf, min_t(int, 16, pc->scsi_cmd->request_bufflen));
 			} else printk("\n");
 		}
 	}
@@ -371,7 +371,7 @@ static int idescsi_end_request (ide_driv
 
 static inline unsigned long get_timeout(idescsi_pc_t *pc)
 {
-	return IDE_MAX(WAIT_CMD, pc->timeout - jiffies);
+	return max_t(long, WAIT_CMD, pc->timeout - jiffies);
 }
 
 /*
@@ -515,7 +515,7 @@ static ide_startstop_t idescsi_issue_pc 
 	scsi->pc=pc;							/* Set the current packet command */
 	pc->actually_transferred=0;					/* We haven't transferred any data yet */
 	pc->current_position=pc->buffer;
-	bcount.all = IDE_MIN(pc->request_transfer, 63 * 1024);		/* Request to transfer the entire buffer at once */
+	bcount.all = min(pc->request_transfer, 63 * 1024);		/* Request to transfer the entire buffer at once */
 
 	feature.all = 0;
 	if (drive->using_dma && rq->bio) {
diff -puN include/linux/ide.h~drivers_ide_minmax2 include/linux/ide.h
--- linux-264-rc2-kj1/include/linux/ide.h~drivers_ide_minmax2	2004-03-08 16:49:15.000000000 -0800
+++ linux-264-rc2-kj1-rddunlap/include/linux/ide.h	2004-03-08 16:49:15.000000000 -0800
@@ -237,8 +237,6 @@ typedef unsigned char	byte;	/* used ever
 #define SECTOR_SIZE	512
 #define SECTOR_WORDS	(SECTOR_SIZE / 4)	/* number of 32bit words per sector */
 #define IDE_LARGE_SEEK(b1,b2,t)	(((b1) > (b2) + (t)) || ((b2) > (b1) + (t)))
-#define IDE_MIN(a,b)	((a)<(b) ? (a):(b))
-#define IDE_MAX(a,b)	((a)>(b) ? (a):(b))
 
 /*
  * Timeouts for various operations:

_

Re: [janitor] use kernel min/max (2/2)

[Bartlomiej Zolnierkiewicz]

> @@ -354,7 +354,7 @@ static int idescsi_end_request (ide_driv
>  			if (!test_bit(PC_WRITING, &pc->flags) && pc->actually_transferred &&
> pc->actually_transferred <= 1024 && pc->buffer) { printk(", rst = ");
>  				scsi_buf = pc->scsi_cmd->request_buffer;
> -				hexdump(scsi_buf, IDE_MIN(16, pc->scsi_cmd->request_bufflen));
> +				hexdump(scsi_buf, min_t(int, 16, pc->scsi_cmd->request_bufflen));
>  			} else printk("\n");
>  		}
>  	}

'unsigned' should be used instead of 'int'
(scsi_cmd->request_bufflen is 'unsigned')

> @@ -371,7 +371,7 @@ static int idescsi_end_request (ide_driv
>
>  static inline unsigned long get_timeout(idescsi_pc_t *pc)
>  {
> -	return IDE_MAX(WAIT_CMD, pc->timeout - jiffies);
> +	return max_t(long, WAIT_CMD, pc->timeout - jiffies);
>  }
>
>  /*

pc->timeout and jiffies are of 'unsigned long' type
so why 'long' is used here?  Timer wrapping protection?

Otherwise patches look okay and I will push them to Linus
as soon as somebody explains this 'long' thing to me. :-)


If somebody is interested there are still related things to fix in IDE code:

drivers/ide/ide-cd.c:
use 'unsigned' instead of 'int' for nskip, sectors_to_transfer, len variables

ide-timing.h:
get rid of MIN()/MAX() macros, this should be done very carefully
as they are used for calculating timings


Regards,
Bartlomiej

Re: [janitor] use kernel min/max (2/2)

[Randy.Dunlap]

On Thu, 18 Mar 2004 18:17:59 +0100 Bartlomiej Zolnierkiewicz wrote:

| 
| > @@ -354,7 +354,7 @@ static int idescsi_end_request (ide_driv
| >  			if (!test_bit(PC_WRITING, &pc->flags) && pc->actually_transferred &&
| > pc->actually_transferred <= 1024 && pc->buffer) { printk(", rst = ");
| >  				scsi_buf = pc->scsi_cmd->request_buffer;
| > -				hexdump(scsi_buf, IDE_MIN(16, pc->scsi_cmd->request_bufflen));
| > +				hexdump(scsi_buf, min_t(int, 16, pc->scsi_cmd->request_bufflen));
| >  			} else printk("\n");
| >  		}
| >  	}
| 
| 'unsigned' should be used instead of 'int'
| (scsi_cmd->request_bufflen is 'unsigned')
| 
| > @@ -371,7 +371,7 @@ static int idescsi_end_request (ide_driv
| >
| >  static inline unsigned long get_timeout(idescsi_pc_t *pc)
| >  {
| > -	return IDE_MAX(WAIT_CMD, pc->timeout - jiffies);
| > +	return max_t(long, WAIT_CMD, pc->timeout - jiffies);
| >  }
| >
| >  /*
| 
| pc->timeout and jiffies are of 'unsigned long' type
| so why 'long' is used here?  Timer wrapping protection?

Yes, I think so.  (see below)

| Otherwise patches look okay and I will push them to Linus
| as soon as somebody explains this 'long' thing to me. :-)

Michael hasn't replied on this AFAIK.

'long' looks correct to me, just based on include/linux/jiffies.h.
In that case, someone has thought lots more about it than I have
and concluded that while jiffies is stored as 'unsigned long',
comparing times is correct when coerced (typecast) to long, as in:

#define time_after(a,b)		\
	(typecheck(unsigned long, a) && \
	 typecheck(unsigned long, b) && \
	 ((long)(b) - (long)(a) < 0))

But if you think it's risky, let's just drop it.

| If somebody is interested there are still related things to fix in IDE code:
| 
| drivers/ide/ide-cd.c:
| use 'unsigned' instead of 'int' for nskip, sectors_to_transfer, len variables
| 
| ide-timing.h:
| get rid of MIN()/MAX() macros, this should be done very carefully
| as they are used for calculating timings


--
~Randy

Re: [janitor] use kernel min/max (2/2)

[Michael Veeck]

Randy.Dunlap wrote:

> On Thu, 18 Mar 2004 18:17:59 +0100 Bartlomiej Zolnierkiewicz wrote:
> 
> | 
> | > @@ -354,7 +354,7 @@ static int idescsi_end_request (ide_driv
> | >  			if (!test_bit(PC_WRITING, &pc->flags) && pc->actually_transferred &&
> | > pc->actually_transferred <= 1024 && pc->buffer) { printk(", rst = ");
> | >  				scsi_buf = pc->scsi_cmd->request_buffer;
> | > -				hexdump(scsi_buf, IDE_MIN(16, pc->scsi_cmd->request_bufflen));
> | > +				hexdump(scsi_buf, min_t(int, 16, pc->scsi_cmd->request_bufflen));
> | >  			} else printk("\n");
> | >  		}
> | >  	}
> | 
> | 'unsigned' should be used instead of 'int'
> | (scsi_cmd->request_bufflen is 'unsigned')
> | 
> | > @@ -371,7 +371,7 @@ static int idescsi_end_request (ide_driv
> | >
> | >  static inline unsigned long get_timeout(idescsi_pc_t *pc)
> | >  {
> | > -	return IDE_MAX(WAIT_CMD, pc->timeout - jiffies);
> | > +	return max_t(long, WAIT_CMD, pc->timeout - jiffies);
> | >  }
> | >
> | >  /*
> | 
> | pc->timeout and jiffies are of 'unsigned long' type
> | so why 'long' is used here?  Timer wrapping protection?
> 
> Yes, I think so.  (see below)
> 
> | Otherwise patches look okay and I will push them to Linus
> | as soon as somebody explains this 'long' thing to me. :-)
> 
> Michael hasn't replied on this AFAIK.

Sorry for being quiet, I'm busy right now looking for a new job.

Anyway, IIRC I choose 'long' because of the jiffies.h too, but I'm 
willing to change that back to 'unsigned long'.

> 
> 'long' looks correct to me, just based on include/linux/jiffies.h.
> In that case, someone has thought lots more about it than I have
> and concluded that while jiffies is stored as 'unsigned long',
> comparing times is correct when coerced (typecast) to long, as in:
> 
> #define time_after(a,b)		\
> 	(typecheck(unsigned long, a) && \
> 	 typecheck(unsigned long, b) && \
> 	 ((long)(b) - (long)(a) < 0))
> 
> But if you think it's risky, let's just drop it.
> 
> | If somebody is interested there are still related things to fix in IDE code:
> | 
> | drivers/ide/ide-cd.c:
> | use 'unsigned' instead of 'int' for nskip, sectors_to_transfer, len variables
> | 
> | ide-timing.h:
> | get rid of MIN()/MAX() macros, this should be done very carefully
> | as they are used for calculating timings

Where's the risk in that? An example would be very helpful.
Thanks for the help

Veeck

> 
> 
> --
> ~Randy
> 
> 

Re: [janitor] use kernel min/max (2/2)

[Bartlomiej Zolnierkiewicz]

On Thursday 22 of April 2004 01:31, Michael Veeck wrote:
> > Michael hasn't replied on this AFAIK.
>
> Sorry for being quiet, I'm busy right now looking for a new job.

No problem.

> Anyway, IIRC I choose 'long' because of the jiffies.h too, but I'm
> willing to change that back to 'unsigned long'.

I'll revert it for now.

> > | ide-timing.h:
> > | get rid of MIN()/MAX() macros, this should be done very carefully
> > | as they are used for calculating timings
>
> Where's the risk in that? An example would be very helpful.

Well... break something and kiss your data good bye... :-)
(If wrong timings get calculated and programmed)

Thanks,
Bartlomiej