Antony Pavlov <antonynpav...@gmail.com> writes:

> apply this commit after that one:
>
>     Author: Robert Jarzmik <robert.jarz...@free.fr>
>     Date:   Sun Nov 4 18:55:23 2012 +0100
>
>         commands: change Y-Modem implementation
> ---
>  lib/xymodem.c |   13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/lib/xymodem.c b/lib/xymodem.c
> index 1469a9a..0e82ce2 100644
> --- a/lib/xymodem.c
> +++ b/lib/xymodem.c
> @@ -257,8 +257,8 @@ static ssize_t xy_read_block(struct xyz_ctxt *proto, 
> struct xy_block *blk,
>       uint64_t timeout)
>  {
>       ssize_t rc, data_len = 0;
> -     unsigned char hdr, seqs[2], crcs[2];
> -     int crc = 0, hdr_found = 0;
> +     unsigned char hdr, seqs[2];
> +     uint16_t crc = 0, hdr_found = 0;
>       uint64_t start = get_time_ns();
>  
>       while (!hdr_found) {
> @@ -308,12 +308,13 @@ static ssize_t xy_read_block(struct xyz_ctxt *proto, 
> struct xy_block *blk,
>  
>       switch (proto->crc_mode) {
>       case CRC_ADD8:
> -             rc = xy_gets(proto->cdev, proto->fifo, crcs, 1, timeout);
> -             crc = crcs[0];
> +             rc = xy_gets(proto->cdev, proto->fifo,
> +                             (unsigned char *)&crc, 1, timeout);
This doesn't look good to me.
In big-endian arch, suppose you read 0xab as the crc.
In that case, won't crc be equal to 0xab00 instead of 0x00ab in this code ? If
that's the case, the previous code was right ...

>               break;
>       case CRC_CRC16:
> -             rc = xy_gets(proto->cdev, proto->fifo, crcs, 2, timeout);
> -             crc = be16_to_cpu(*(uint16_t *)crcs);
> +             rc = xy_gets(proto->cdev, proto->fifo,
> +                             (unsigned char *)&crc, 2, timeout);
> +             crc = be16_to_cpu(crc);

Does that mean that the code up there doesn't work on big endian ? I'm really
puzzled, as the sender sends in big endian, the xy_gets() receives the bytes
into crcs[2], and the be16_to_cpu() should be a no-op ...

I think that we should do this simpler :
  crc = crcs[0] << 8 + crcs[1]

I'll put that into my github branch for testing.

Cheers.

-- 
Robert

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Reply via email to