lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Sun, 21 Dec 2008 20:15:37 +0100
From:	Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
To:	Borislav Petkov <petkovbb@...glemail.com>
Cc:	linux-kernel@...r.kernel.org, linux-ide@...r.kernel.org,
	Borislav Petkov <petkovbb@...il.com>
Subject: Re: [PATCH 5/8] ide-cd: remove handler wrappers

On Thursday 18 December 2008, Borislav Petkov wrote:
> Remove cdrom_do_newpc_cont and cdrom_start_rw_cont wrappers and pass
> cdrom_transfer_packet_command to ide_execute_command directly.
> 
> There should be no functionality change resulting from this patch.
> 
> Signed-off-by: Borislav Petkov <petkovbb@...il.com>
> ---
>  drivers/ide/ide-cd.c |  121 ++++++++++++++++++++------------------------------
>  1 files changed, 48 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
> index 34981f5..f3c7cc3 100644
> --- a/drivers/ide/ide-cd.c
> +++ b/drivers/ide/ide-cd.c
> @@ -511,48 +511,7 @@ end_request:
>  	return 1;
>  }
>  
> -/*
> - * Set up the device registers for transferring a packet command on DEV,
> - * expecting to later transfer XFERLEN bytes.  HANDLER is the routine
> - * which actually transfers the command to the drive.  If this is a
> - * drq_interrupt device, this routine will arrange for HANDLER to be
> - * called when the interrupt from the drive arrives.  Otherwise, HANDLER
> - * will be called immediately after the drive is prepared for the transfer.
> - */
> -static ide_startstop_t cdrom_start_packet_command(ide_drive_t *drive,
> -						  ide_handler_t *handler)
> -{
> -	ide_hwif_t *hwif = drive->hwif;
> -	struct request *rq = hwif->hwgroup->rq;
> -	int xferlen;
> -
> -	xferlen = ide_cd_get_xferlen(rq);
> -
> -	ide_debug_log(IDE_DBG_PC, "Call %s, xferlen: %d\n", __func__, xferlen);
> -
> -	/* FIXME: for Virtual DMA we must check harder */
> -	if (drive->dma)
> -		drive->dma = !hwif->dma_ops->dma_setup(drive);
> -
> -	/* set up the controller registers */
> -	ide_pktcmd_tf_load(drive, IDE_TFLAG_OUT_NSECT | IDE_TFLAG_OUT_LBAL,
> -			   xferlen, drive->dma);
> -
> -	if (drive->atapi_flags & IDE_AFLAG_DRQ_INTERRUPT) {
> -		/* waiting for CDB interrupt, not DMA yet. */
> -		if (drive->dma)
> -			drive->waiting_for_dma = 0;
> -
> -		/* packet command */
> -		ide_execute_command(drive, ATA_CMD_PACKET, handler,
> -				    ATAPI_WAIT_PC, ide_cd_expiry);
> -		return ide_started;
> -	} else {
> -		ide_execute_pkt_cmd(drive);
> -
> -		return (*handler) (drive);
> -	}
> -}
> +static ide_startstop_t cdrom_newpc_intr(ide_drive_t *);
>  
>  /*
>   * Send a packet command to DRIVE described by CMD_BUF and CMD_LEN. The device
> @@ -561,11 +520,10 @@ static ide_startstop_t cdrom_start_packet_command(ide_drive_t *drive,
>   * there's data ready.
>   */
>  #define ATAPI_MIN_CDB_BYTES 12
> -static ide_startstop_t cdrom_transfer_packet_command(ide_drive_t *drive,
> -					  struct request *rq,
> -					  ide_handler_t *handler)
> +static ide_startstop_t cdrom_transfer_packet_command(ide_drive_t *drive)
>  {
>  	ide_hwif_t *hwif = drive->hwif;
> +	struct request *rq = hwif->hwgroup->rq;
>  	int cmd_len;
>  	ide_startstop_t startstop;
>  
> @@ -592,7 +550,7 @@ static ide_startstop_t cdrom_transfer_packet_command(ide_drive_t *drive,
>  	}
>  
>  	/* arm the interrupt handler */
> -	ide_set_handler(drive, handler, rq->timeout, ide_cd_expiry);
> +	ide_set_handler(drive, cdrom_newpc_intr, rq->timeout, ide_cd_expiry);
>  
>  	/* ATAPI commands get padded out to 12 bytes minimum */
>  	cmd_len = COMMAND_SIZE(rq->cmd[0]);
> @@ -610,6 +568,49 @@ static ide_startstop_t cdrom_transfer_packet_command(ide_drive_t *drive,
>  }
>  
>  /*
> + * Set up the device registers for transferring a packet command on DEV,
> + * expecting to later transfer XFERLEN bytes.  HANDLER is the routine
> + * which actually transfers the command to the drive.  If this is a
> + * drq_interrupt device, this routine will arrange for HANDLER to be
> + * called when the interrupt from the drive arrives.  Otherwise, HANDLER
> + * will be called immediately after the drive is prepared for the transfer.
> + */
> +static ide_startstop_t cdrom_start_packet_command(ide_drive_t *drive)
> +{
> +	ide_hwif_t *hwif = drive->hwif;
> +	struct request *rq = hwif->hwgroup->rq;
> +	int xferlen;
> +
> +	xferlen = ide_cd_get_xferlen(rq);
> +
> +	ide_debug_log(IDE_DBG_PC, "Call %s, xferlen: %d\n", __func__, xferlen);
> +
> +	/* FIXME: for Virtual DMA we must check harder */
> +	if (drive->dma)
> +		drive->dma = !hwif->dma_ops->dma_setup(drive);
> +
> +	/* set up the controller registers */
> +	ide_pktcmd_tf_load(drive, IDE_TFLAG_OUT_NSECT | IDE_TFLAG_OUT_LBAL,
> +			   xferlen, drive->dma);
> +
> +	if (drive->atapi_flags & IDE_AFLAG_DRQ_INTERRUPT) {
> +		/* waiting for CDB interrupt, not DMA yet. */
> +		if (drive->dma)
> +			drive->waiting_for_dma = 0;
> +
> +		/* packet command */
> +		ide_execute_command(drive, ATA_CMD_PACKET,
> +				    cdrom_transfer_packet_command,
> +				    ATAPI_WAIT_PC, ide_cd_expiry);
> +		return ide_started;
> +	} else {
> +		ide_execute_pkt_cmd(drive);
> +
> +		return cdrom_transfer_packet_command(drive);
> +	}
> +}
> +
> +/*
>   * Check the contents of the interrupt reason register from the cdrom
>   * and attempt to recover if there are problems.  Returns  0 if everything's
>   * ok; nonzero if the request has been terminated.
> @@ -680,8 +681,6 @@ static int ide_cd_check_transfer_size(ide_drive_t *drive, int len)
>  	return 1;
>  }
>  
> -static ide_startstop_t cdrom_newpc_intr(ide_drive_t *);
> -
>  static ide_startstop_t ide_cd_prepare_rw_request(ide_drive_t *drive,
>  						 struct request *rq)
>  {
> @@ -724,20 +723,6 @@ static ide_startstop_t ide_cd_prepare_rw_request(ide_drive_t *drive,
>  }
>  
>  /*
> - * Routine to send a read/write packet command to the drive. This is usually
> - * called directly from cdrom_start_{read,write}(). However, for drq_interrupt
> - * devices, it is called from an interrupt when the drive is ready to accept
> - * the command.
> - */
> -static ide_startstop_t cdrom_start_rw_cont(ide_drive_t *drive)
> -{
> -	struct request *rq = drive->hwif->hwgroup->rq;
> -
> -	/* send the command to the drive and return */
> -	return cdrom_transfer_packet_command(drive, rq, cdrom_newpc_intr);
> -}
> -
> -/*
>   * Fix up a possibly partially-processed request so that we can start it over
>   * entirely, or even put it back on the request queue.
>   */
> @@ -1126,13 +1111,6 @@ static ide_startstop_t cdrom_start_rw(ide_drive_t *drive, struct request *rq)
>  	return ide_started;
>  }
>  
> -static ide_startstop_t cdrom_do_newpc_cont(ide_drive_t *drive)
> -{
> -	struct request *rq = HWGROUP(drive)->rq;
> -
> -	return cdrom_transfer_packet_command(drive, rq, cdrom_newpc_intr);
> -}
> -
>  static void cdrom_do_block_pc(ide_drive_t *drive, struct request *rq)
>  {
>  
> @@ -1177,7 +1155,6 @@ static void cdrom_do_block_pc(ide_drive_t *drive, struct request *rq)
>  static ide_startstop_t ide_cd_do_request(ide_drive_t *drive, struct request *rq,
>  					sector_t block)
>  {
> -	ide_handler_t *fn;
>  
>  	ide_debug_log(IDE_DBG_RQ, "Call %s, rq->cmd[0]: 0x%x, "
>  		      "rq->cmd_type: 0x%x, block: %llu\n",
> @@ -1185,7 +1162,6 @@ static ide_startstop_t ide_cd_do_request(ide_drive_t *drive, struct request *rq,
>  		      (unsigned long long)block);
>  
>  	if (blk_fs_request(rq)) {
> -		fn = cdrom_start_rw_cont;
>  
>  		if (cdrom_start_rw(drive, rq) == ide_stopped)
>  			return ide_stopped;
> @@ -1194,7 +1170,6 @@ static ide_startstop_t ide_cd_do_request(ide_drive_t *drive, struct request *rq,
>  			return ide_stopped;
>  	} else if (blk_sense_request(rq) || blk_pc_request(rq) ||
>  		   rq->cmd_type == REQ_TYPE_ATA_PC) {
> -		fn = cdrom_do_newpc_cont;
>  
>  		if (!rq->timeout)
>  			rq->timeout = ATAPI_WAIT_PC;
> @@ -1210,7 +1185,7 @@ static ide_startstop_t ide_cd_do_request(ide_drive_t *drive, struct request *rq,
>  		return ide_stopped;
>  	}
>  
> -	return cdrom_start_packet_command(drive, fn);
> +	return cdrom_start_packet_command(drive);
>  }

There is no need to move cdrom_start_packet_command() around (especially
since it goes away in patch #9/8) and not doing so allows to see changes
to the function immediately.  Fixed in the merged patch version:

From: Borislav Petkov <petkovbb@...glemail.com>
Subject: [PATCH 5/8] ide-cd: remove handler wrappers

Remove cdrom_do_newpc_cont and cdrom_start_rw_cont wrappers and pass
cdrom_transfer_packet_command to ide_execute_command directly.

There should be no functionality change resulting from this patch.

Signed-off-by: Borislav Petkov <petkovbb@...il.com>
[bart: don't move cdrom_start_packet_command() around, remove newlines]
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
---
 drivers/ide/ide-cd.c |   49 +++++++++++--------------------------------------
 1 file changed, 11 insertions(+), 38 deletions(-)

Index: b/drivers/ide/ide-cd.c
===================================================================
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -511,6 +511,9 @@ end_request:
 	return 1;
 }
 
+static ide_startstop_t cdrom_transfer_packet_command(ide_drive_t *);
+static ide_startstop_t cdrom_newpc_intr(ide_drive_t *);
+
 /*
  * Set up the device registers for transferring a packet command on DEV,
  * expecting to later transfer XFERLEN bytes.  HANDLER is the routine
@@ -519,8 +522,7 @@ end_request:
  * called when the interrupt from the drive arrives.  Otherwise, HANDLER
  * will be called immediately after the drive is prepared for the transfer.
  */
-static ide_startstop_t cdrom_start_packet_command(ide_drive_t *drive,
-						  ide_handler_t *handler)
+static ide_startstop_t cdrom_start_packet_command(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif = drive->hwif;
 	struct request *rq = hwif->hwgroup->rq;
@@ -544,13 +546,14 @@ static ide_startstop_t cdrom_start_packe
 			drive->waiting_for_dma = 0;
 
 		/* packet command */
-		ide_execute_command(drive, ATA_CMD_PACKET, handler,
+		ide_execute_command(drive, ATA_CMD_PACKET,
+				    cdrom_transfer_packet_command,
 				    ATAPI_WAIT_PC, ide_cd_expiry);
 		return ide_started;
 	} else {
 		ide_execute_pkt_cmd(drive);
 
-		return (*handler) (drive);
+		return cdrom_transfer_packet_command(drive);
 	}
 }
 
@@ -561,11 +564,10 @@ static ide_startstop_t cdrom_start_packe
  * there's data ready.
  */
 #define ATAPI_MIN_CDB_BYTES 12
-static ide_startstop_t cdrom_transfer_packet_command(ide_drive_t *drive,
-					  struct request *rq,
-					  ide_handler_t *handler)
+static ide_startstop_t cdrom_transfer_packet_command(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif = drive->hwif;
+	struct request *rq = hwif->hwgroup->rq;
 	int cmd_len;
 	ide_startstop_t startstop;
 
@@ -592,7 +594,7 @@ static ide_startstop_t cdrom_transfer_pa
 	}
 
 	/* arm the interrupt handler */
-	ide_set_handler(drive, handler, rq->timeout, ide_cd_expiry);
+	ide_set_handler(drive, cdrom_newpc_intr, rq->timeout, ide_cd_expiry);
 
 	/* ATAPI commands get padded out to 12 bytes minimum */
 	cmd_len = COMMAND_SIZE(rq->cmd[0]);
@@ -680,8 +682,6 @@ static int ide_cd_check_transfer_size(id
 	return 1;
 }
 
-static ide_startstop_t cdrom_newpc_intr(ide_drive_t *);
-
 static ide_startstop_t ide_cd_prepare_rw_request(ide_drive_t *drive,
 						 struct request *rq)
 {
@@ -724,20 +724,6 @@ static ide_startstop_t ide_cd_prepare_rw
 }
 
 /*
- * Routine to send a read/write packet command to the drive. This is usually
- * called directly from cdrom_start_{read,write}(). However, for drq_interrupt
- * devices, it is called from an interrupt when the drive is ready to accept
- * the command.
- */
-static ide_startstop_t cdrom_start_rw_cont(ide_drive_t *drive)
-{
-	struct request *rq = drive->hwif->hwgroup->rq;
-
-	/* send the command to the drive and return */
-	return cdrom_transfer_packet_command(drive, rq, cdrom_newpc_intr);
-}
-
-/*
  * Fix up a possibly partially-processed request so that we can start it over
  * entirely, or even put it back on the request queue.
  */
@@ -1126,13 +1112,6 @@ static ide_startstop_t cdrom_start_rw(id
 	return ide_started;
 }
 
-static ide_startstop_t cdrom_do_newpc_cont(ide_drive_t *drive)
-{
-	struct request *rq = HWGROUP(drive)->rq;
-
-	return cdrom_transfer_packet_command(drive, rq, cdrom_newpc_intr);
-}
-
 static void cdrom_do_block_pc(ide_drive_t *drive, struct request *rq)
 {
 
@@ -1177,16 +1156,12 @@ static void cdrom_do_block_pc(ide_drive_
 static ide_startstop_t ide_cd_do_request(ide_drive_t *drive, struct request *rq,
 					sector_t block)
 {
-	ide_handler_t *fn;
-
 	ide_debug_log(IDE_DBG_RQ, "Call %s, rq->cmd[0]: 0x%x, "
 		      "rq->cmd_type: 0x%x, block: %llu\n",
 		      __func__, rq->cmd[0], rq->cmd_type,
 		      (unsigned long long)block);
 
 	if (blk_fs_request(rq)) {
-		fn = cdrom_start_rw_cont;
-
 		if (cdrom_start_rw(drive, rq) == ide_stopped)
 			return ide_stopped;
 
@@ -1194,8 +1169,6 @@ static ide_startstop_t ide_cd_do_request
 			return ide_stopped;
 	} else if (blk_sense_request(rq) || blk_pc_request(rq) ||
 		   rq->cmd_type == REQ_TYPE_ATA_PC) {
-		fn = cdrom_do_newpc_cont;
-
 		if (!rq->timeout)
 			rq->timeout = ATAPI_WAIT_PC;
 
@@ -1210,7 +1183,7 @@ static ide_startstop_t ide_cd_do_request
 		return ide_stopped;
 	}
 
-	return cdrom_start_packet_command(drive, fn);
+	return cdrom_start_packet_command(drive);
 }
 
 /*
.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ