linux-kernel - Re: [PATCH] mflash: remove small byteswapping function

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]

Message-ID: <57afda040901200025q6bc1bab1id9a1e6d6eac86d17@mail.gmail.com>
Date:	Tue, 20 Jan 2009 17:25:56 +0900
From:	"unsik Kim" <donari75@...il.com>
To:	"Harvey Harrison" <harvey.harrison@...il.com>
Cc:	"Heikki Orsila" <shdl@...alwe.fi>, linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org, "Alan Cox" <alan@...rguk.ukuu.org.uk>
Subject: Re: [PATCH] mflash: remove small byteswapping function

Thank you for suggestions.

I removed mg_fix_disk_id and remove byteswap at mg_ide_fixstring.

I attached fixed code.

2009/1/6 Harvey Harrison <harvey.harrison@...il.com>:
> Byteswap as we read in the data rather than having a
> byteswap in place function that is only used once.
>
> MG_SECTOR_SIZE >> 1 == 256 so this should be a no-op.
>
> Signed-off-by: Harvey Harrison <harvey.harrison@...il.com>
> ---
> Also, you always call mg_ide_fixstring with byteswap == 1,
> I'd suggest removing that parameter and doing it unconditionally.
>
>  drivers/block/mg_disk.c |   17 +----------------
>  1 files changed, 1 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/block/mg_disk.c b/drivers/block/mg_disk.c
> index 24edc9a..116f2a3 100644
> --- a/drivers/block/mg_disk.c
> +++ b/drivers/block/mg_disk.c
> @@ -141,20 +141,6 @@ static irqreturn_t mg_irq(int irq, void *dev_id)
>        return IRQ_HANDLED;
>  }
>
> -static void mg_fix_disk_id(u16 *id)
> -{
> -#ifndef __LITTLE_ENDIAN
> -# ifdef __BIG_ENDIAN
> -       int i;
> -
> -       for (i = 0; i < 256; i++)
> -               id[i] = __le16_to_cpu(id[i]);
> -# else
> -#  error "Please fix <asm/byteorder.h>"
> -# endif
> -#endif
> -}
> -
>  static void mg_ide_fixstring(u8 *s, const int bytecount, const int byteswap)
>  {
>        u8 *p, *end = &s[bytecount & ~1]; /* bytecount must be even */
> @@ -194,12 +180,11 @@ static int mg_get_disk_id(struct mg_host *host)
>        err = mg_wait(host, MG_REG_STATUS_BIT_DATA_REQ, 3000);
>        if (!err) {
>                for (i = 0; i < (MG_SECTOR_SIZE >> 1); i++)
> -                       id[i] = inw(host->dev_base + MG_BUFF_OFFSET + i * 2);
> +                       id[i] = le16_to_cpu(inw(host->dev_base + MG_BUFF_OFFSET + i * 2));
>
>                outb(MG_CMD_RD_CONF, host->dev_base + MG_REG_COMMAND);
>                err = mg_wait(host, MG_STAT_READY, 3000);
>                if (!err) {
> -                       mg_fix_disk_id(id);
>                        if ((host->id_data.field_valid & 1) == 0) {
>                                err = MG_ERR_TRANSLATION;
>                        } else {
> --
> 1.6.1.71.gd5e04
>
>
>
>

Download attachment "mg_disk.patch" of type "application/octet-stream" (28775 bytes)