Re: [librsync-commits] librsync emit.c,1.46,1.47

G'day,

nice to see work on librsync again... 

On Fri, Sep 10, 2004 at 01:10:01AM +0000, Martin Pool wrote:
[...]
> Problem: rs_emit_copy_cmd used to use the RS_OP_COPY_N4_N4 format by
> default, but this causes integer truncation when one of the operands
> is over 32 bits.
> 
> Fix: Use the right encoding for any operand sizes.
[...]
> Index: emit.c
> ===================================================================
> RCS file: /cvsroot/librsync/librsync/emit.c,v
[...]
> ***************
> *** 104,119 ****
>       int            cmd;
>       rs_stats_t     *stats = &job->stats;
> !     int where_bytes = rs_int_len(where);
> !     int len_bytes   = rs_int_len(len);
>   
> !     switch (10 * where_bytes + len_bytes) {
> !         case 88 :   cmd = RS_OP_COPY_N8_N8; break;
> !         case 84 :   cmd = RS_OP_COPY_N8_N4; break;
> !         case 48 :   cmd = RS_OP_COPY_N4_N8; break;
> !         default :   cmd = RS_OP_COPY_N4_N4; 
> !                     where_bytes = 4; 
> !                     len_bytes   = 4;
>       }
>   
>       rs_trace("emit COPY_N%d_N%d(where=%.0f, len=%.0f), cmd_byte=%#x",
>                where_bytes, len_bytes, (double) where, (double) len, cmd);
> --- 107,140 ----
>       int            cmd;
>       rs_stats_t     *stats = &job->stats;
> !     const int where_bytes = rs_int_len(where);
> !     const int len_bytes   = rs_int_len(len);
>   
> !     /* Commands ascend (1,1), (1,2), ... (8, 8) */
> !     if (where_bytes == 8) 
> !         cmd = RS_OP_COPY_N8_N1;
> !     else if (where_bytes == 4)
> !         cmd = RS_OP_COPY_N4_N1;
> !     else if (where_bytes == 2)
> !         cmd = RS_OP_COPY_N2_N1;
> !     else if (where_bytes == 1)
> !         cmd = RS_OP_COPY_N1_N1;
> !     else {
> !         rs_fatal("can't encode copy command with where_bytes=%d",
> !                  where_bytes);
>       }
>   
> +     if (len_bytes == 1)
> +         ;
> +     else if (len_bytes == 2)
> +         cmd += 1;
> +     else if (len_bytes == 4)
> +         cmd += 2;
> +     else if (len_bytes == 8)
> +         cmd += 3;
> +     else {
> +         rs_fatal("can't encode copy command with len_bytes=%d",
> +                  len_bytes);
> +     }       
> + 
>       rs_trace("emit COPY_N%d_N%d(where=%.0f, len=%.0f), cmd_byte=%#x",
>                where_bytes, len_bytes, (double) where, (double) len, cmd);

I would have thought extending the case statement to cover all options
would have been simpler and more efficient. 

Is there a reason it was done this way? (compilers can be weird).

However... he who does it, decides how it gets done. Thanks for the
fix.

-- 
----------------------------------------------------------------
Donovan Baarda                http://minkirri.apana.org.au/~abo/
----------------------------------------------------------------


-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM. 
Deadline: Sept. 13. Go here: http://sf.net/ppc_contest.php