linux-kernel - Re: [RESEND PATCH 06/10] ide-floppy: report DMA handling in idefloppy_pc

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 5 Jan 2008 16:46:05 +0100
From:	Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
To:	Borislav Petkov <bbpetkov@...oo.de>
Cc:	linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RESEND PATCH 06/10] ide-floppy: report DMA handling in idefloppy_pc_intr() properly


On Thursday 03 January 2008, Borislav Petkov wrote:
> while at it:
> -cleanup small issues raised by scripts/checkpatch.pl
> -remove redundant and shorten some comments

Could you split the above coding style fixes into a separate patch?

> There should be no functionality change resulting from this patch

The patch fixes the DMA error reporting so this is not true. :)

> Signed-off-by: Borislav Petkov <bbpetkov@...oo.de>
> ---
>  drivers/ide/ide-floppy.c |  195 ++++++++++++++++++++--------------------------
>  1 files changed, 83 insertions(+), 112 deletions(-)
> 
> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> index 7823447..5d0fd1f 100644
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c
> @@ -5,9 +5,7 @@
>   * Copyright (C) 2000 - 2002 Paul Bristow <paul@...lbristow.net>
>   *
>   * The driver currently doesn't have any fancy features, just the bare
> - * minimum read/write support.

this comment should be removed as it is not true anymore
(SG_IO and IDEFLOPPY_IOCTL_FORMAT_* ioctls are supported)

[...]

> @@ -208,11 +204,11 @@ static void idefloppy_update_buffers (ide_drive_t *drive, idefloppy_pc_t *pc)
>  }
>  
>  /*
> - *	idefloppy_queue_pc_head generates a new packet command request in front
> - *	of the request queue, before the current request, so that it will be
> - *	processed immediately, on the next pass through the driver.
> + * Generates a new packet command request in front

a lot of space left in the above line

> + * of the request queue, before the current request, so that it will be
> + * processed immediately, on the next pass through the driver.
>   */
> -static void idefloppy_queue_pc_head (ide_drive_t *drive,idefloppy_pc_t *pc,
> +static void idefloppy_queue_pc_head(ide_drive_t *drive, idefloppy_pc_t *pc,
>  		struct request *rq)
>  {
>  	struct ide_floppy_obj *floppy = drive->driver_data;

[...]

> @@ -554,11 +545,7 @@ static ide_startstop_t idefloppy_transfer_pc1 (ide_drive_t *drive)
>  	return ide_started;
>  }
>  
> -/**
> - * idefloppy_should_report_error()
> - *
> - * Supresses error messages resulting from Medium not present
> - */
> +/* Supresses error messages resulting from Medium not present */

missing '.'

> @@ -792,7 +774,8 @@ static void idefloppy_create_rw_cmd (idefloppy_t *floppy, idefloppy_pc_t *pc,
>  }
>  
>  static void
> -idefloppy_blockpc_cmd(idefloppy_t *floppy, idefloppy_pc_t *pc, struct request *rq)
> +idefloppy_blockpc_cmd(idefloppy_t *floppy, idefloppy_pc_t *pc,
> +		struct request *rq)

might as well be converted to match the coding style:

static void idefloppy_blockpc_cmd...

[...]

> @@ -1437,7 +1408,7 @@ static void idefloppy_setup (ide_drive_t *drive, idefloppy_t *floppy)
>  	idefloppy_add_settings(drive);
>  }
>  
> -static void ide_floppy_remove(ide_drive_t *drive)
> +static void idefloppy_remove(ide_drive_t *drive)

Hmm, no.  The driver is called ide-floppy (ide_floppy) and it is more
readable this way.

>  {
>  	idefloppy_t *floppy = drive->driver_data;
>  	struct gendisk *g = floppy->disk;
> @@ -1479,7 +1450,7 @@ static ide_proc_entry_t idefloppy_proc[] = {
>  };
>  #endif	/* CONFIG_IDE_PROC_FS */
>  
> -static int ide_floppy_probe(ide_drive_t *);
> +static int idefloppy_probe(ide_drive_t *);

ditto
 
[...]

> @@ -1733,7 +1704,7 @@ static struct block_device_operations idefloppy_ops = {
>  	.revalidate_disk= idefloppy_revalidate_disk
>  };
>  
> -static int ide_floppy_probe(ide_drive_t *drive)
> +static int idefloppy_probe(ide_drive_t *drive)

ditto

otherwise looks good
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/