[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-trivial] [PATCH v1 1/5] block/nand: Factor out co
From: |
Michael Tokarev |
Subject: |
Re: [Qemu-devel] [Qemu-trivial] [PATCH v1 1/5] block/nand: Factor out common code |
Date: |
Wed, 19 Jun 2013 12:09:57 +0400 |
User-agent: |
Mozilla/5.0 (X11; Linux i686 on x86_64; rv:17.0) Gecko/17.0 Icedove/17.0 |
18.06.2013 15:08, address@hidden wrote:
> From: Peter Crosthwaite <address@hidden>
>
> Most of this computation of s->iolen is the same for both the if and
> else paths here. Factor out the common parts outside the if.
>
> Cc: address@hidden
>
> Signed-off-by: Peter Crosthwaite <address@hidden>
> ---
>
> hw/block/nand.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/block/nand.c b/hw/block/nand.c
> index 087ca14..6309f93 100644
> --- a/hw/block/nand.c
> +++ b/hw/block/nand.c
> @@ -272,10 +272,10 @@ static void nand_command(NANDFlashState *s)
> break;
> offset = s->addr & ((1 << s->addr_shift) - 1);
> s->blk_load(s, s->addr, offset);
> - if (s->gnd)
> - s->iolen = (1 << s->page_shift) - offset;
> - else
> - s->iolen = (1 << s->page_shift) + (1 << s->oob_shift) - offset;
> + s->iolen = (1 << s->page_shift) - offset;
> + if (!s->gnd) {
> + s->iolen += 1 << s->oob_shift;
> + }
Hm. Can s->iolen become negative here?
addr_shift can be either less or greather than page_shift. When addr_shift
is larger than page_shift (addr_shift=16, page_shift=11), offset may be up
to 65535, in which case s->iolen may be -63487, even without additional
oob_shift.
I dunno if this is a concern, just.. asking.
Besides, exactly the same expression is used in nand_getio() down this file,
maybe a common function is in order? :)
And looking at this file, I think it deserves some good type changes. For
example, all these _shift are unsigned, and most are actually used as masks,
in form (1<<shift), instead of directly (except of erase_shift).
Thanks,
/mjt
- [Qemu-devel] [PATCH v1 0/5] Nand Cleanup, peter . crosthwaite, 2013/06/18
- [Qemu-devel] [PATCH v1 2/5] block/nand: Formatting sweep, peter . crosthwaite, 2013/06/18
- [Qemu-devel] [PATCH v1 3/5] block/nand: QOM casting sweep, peter . crosthwaite, 2013/06/18
- [Qemu-devel] [PATCH v1 4/5] block/nand: Convert Sysbus::init to Device::realize, peter . crosthwaite, 2013/06/18
- [Qemu-devel] [PATCH v1 5/5] nand: Don't inherit from Sysbus, peter . crosthwaite, 2013/06/18