WBX,

>    Attached is a patch to optimize wlbc in current branch. Only one 
> allocation of memroy, not two.

Thank you for the patch. Please find hereafter some inline
comments/questions on your patch.

And, I changed the subject of the email thread to better describe its
topic.


>  struct c_window
>  {
> +     uint16_t is_used;    /**< Whether the window entry is used
> or not */ uint16_t sn;     /**< The Sequence Number (SN) associated
> with the entry (used to acknowledge the entry) */
>       uint32_t value;  /**< The value stored in the window entry */
> -     bool is_used;    /**< Whether the window entry is used or
> not */ };

Why changing the position and the type of is_used?


>       size_t bits;
>       /// Shift parameter (see 4.5.2 in the RFC 3095)
>       rohc_lsb_shift_t p;
> +
> +     /// @brief The window in which numerous previous values of
> the encoded value
> +     ///        are stored to help recreate the value
> +     struct c_window window[1];
>  };

Why not use flexible arrays from ISO C99?

You may find documentation in:
http://gcc.gnu.org/onlinedocs/gcc-4.7.0/gcc/Zero-Length.html


> -     wlsb = malloc(sizeof(struct c_wlsb));
> +     wlsb = malloc(sizeof(struct c_wlsb)+(sizeof(struct
> c_window)*(window_width-1))); if(wlsb == NULL)
>       {
>               rohc_debugf(0, "cannot allocate memory for the W-LSB
> object\n"); goto error;
>       }
> -     bzero(wlsb, sizeof(struct c_wlsb));

OK. At first, I was not sure about the bzero() removal, but all
required struct members are initialized, so that's OK.


Regards,
Didier

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Mailing list: https://launchpad.net/~rohc
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~rohc
More help   : https://help.launchpad.net/ListHelp

Reply via email to