[PATCH] lzo: fix possible typo in decompresor - Kernel

This is a discussion on [PATCH] lzo: fix possible typo in decompresor - Kernel ; Shift of a le value seems strange, probably meant to shift the cpu-order variable as in the prvious section of the switch statement. Signed-off-by: Harvey Harrison --- lib/lzo/lzo1x_decompress.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git ...

+ Reply to Thread
Results 1 to 5 of 5

Thread: [PATCH] lzo: fix possible typo in decompresor

  1. [PATCH] lzo: fix possible typo in decompresor

    Shift of a le value seems strange, probably meant to shift the cpu-order
    variable as in the prvious section of the switch statement.

    Signed-off-by: Harvey Harrison
    ---
    lib/lzo/lzo1x_decompress.c | 2 +-
    1 files changed, 1 insertions(+), 1 deletions(-)

    diff --git a/lib/lzo/lzo1x_decompress.c b/lib/lzo/lzo1x_decompress.c
    index 9dc7056..77f0f9b 100644
    --- a/lib/lzo/lzo1x_decompress.c
    +++ b/lib/lzo/lzo1x_decompress.c
    @@ -158,7 +158,7 @@ match:
    t += 7 + *ip++;
    }
    m_pos -= le16_to_cpu(get_unaligned(
    - (const unsigned short *)ip) >> 2);
    + (const unsigned short *)ip)) >> 2;
    ip += 2;
    if (m_pos == op)
    goto eof_found;
    --
    1.5.5.144.g3e42



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

  2. Re: [PATCH] lzo: fix possible typo in decompresor



    On Thu, 10 Apr 2008, Harvey Harrison wrote:
    >
    > Shift of a le value seems strange, probably meant to shift the cpu-order
    > variable as in the prvious section of the switch statement.


    Hmm. This patch looks ObviouslyCorrect(tm), but it worries me that
    apparently the old broken code has been around since last July, and afaik
    it can never have worked on big-endian machines.

    So did nobody ever use it, or why hasn't this ever triggered? How did you
    find this? A sparse warning?

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

  3. Re: [PATCH] lzo: fix possible typo in decompresor

    On Thu, 2008-04-10 at 13:49 -0700, Linus Torvalds wrote:
    >
    > On Thu, 10 Apr 2008, Harvey Harrison wrote:
    > >
    > > Shift of a le value seems strange, probably meant to shift the cpu-order
    > > variable as in the prvious section of the switch statement.

    >
    > Hmm. This patch looks ObviouslyCorrect(tm), but it worries me that
    > apparently the old broken code has been around since last July, and afaik
    > it can never have worked on big-endian machines.
    >
    > So did nobody ever use it, or why hasn't this ever triggered? How did you
    > find this? A sparse warning?
    >
    > Linus


    When converting in tree users to my proposed unaligned byteswapping api.
    See the 8 patch series posted today to linux-arch.

    Harvey

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

  4. Re: [PATCH] lzo: fix possible typo in decompresor

    On Thu, 2008-04-10 at 13:49 -0700, Linus Torvalds wrote:
    > On Thu, 10 Apr 2008, Harvey Harrison wrote:
    > >
    > > Shift of a le value seems strange, probably meant to shift the cpu-order
    > > variable as in the prvious section of the switch statement.

    >
    > Hmm. This patch looks ObviouslyCorrect(tm), but it worries me that
    > apparently the old broken code has been around since last July, and afaik
    > it can never have worked on big-endian machines.
    >
    > So did nobody ever use it, or why hasn't this ever triggered? How did you
    > find this? A sparse warning?


    The heaviest users of the lzo code I know of are little-endian ARM
    devices through jffs2. When the code was merged there was a lot of
    discussion about the best way to handle the endian issues and unaligned
    accesses and whilst I seem to remember someone posting big-endian test
    results it could have been before some of the later changes were made.

    So yes its possible its not been run on BE until now or that isn't a
    common code path. I've checked this against the external LZO library its
    based on and the patch is correct

    Acked-by: Richard Purdie

    --
    Richard


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

  5. Re: [PATCH] lzo: fix possible typo in decompresor

    On Thu, 2008-04-10 at 23:18 +0100, Richard Purdie wrote:
    > On Thu, 2008-04-10 at 13:49 -0700, Linus Torvalds wrote:
    > > On Thu, 10 Apr 2008, Harvey Harrison wrote:
    > > >
    > > > Shift of a le value seems strange, probably meant to shift the cpu-order
    > > > variable as in the prvious section of the switch statement.

    > >
    > > Hmm. This patch looks ObviouslyCorrect(tm), but it worries me that
    > > apparently the old broken code has been around since last July, and afaik
    > > it can never have worked on big-endian machines.
    > >
    > > So did nobody ever use it, or why hasn't this ever triggered? How did you
    > > find this? A sparse warning?

    >
    > The heaviest users of the lzo code I know of are little-endian ARM
    > devices through jffs2. When the code was merged there was a lot of
    > discussion about the best way to handle the endian issues and unaligned
    > accesses and whilst I seem to remember someone posting big-endian test
    > results it could have been before some of the later changes were made.
    >
    > So yes its possible its not been run on BE until now or that isn't a
    > common code path. I've checked this against the external LZO library its
    > based on and the patch is correct
    >
    > Acked-by: Richard Purdie
    >


    This is one of the reasons I thought about for adding the new api, the
    bracketing is just too easy to get wrong when you throw
    get/put_unaligned into the mix.

    Harvey

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

+ Reply to Thread