rohc team mailing list archive

[RoHC Team <rohc_team@xxxxxxxxxx>]

 Hello Didier,

 Find my answers bellow in the text.

 Sorry for the subject of my initial e-mail, I used the last one by 
facility ...

 Best Regards,

             FWX.

Le 24/05/2012 21:39, Didier Barvaux a écrit :
> 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?

==> Just to align members of the structure, and compact
   the size (16+16+32) rather than (16+(16)+32+32) when not compacted.

> >   	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?

==> Because I am not sure all old compilers understand and support that, 
especially older cross-compilers I used. This way is not the best, but I 
am sure that it is very easy to compile.

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

==> Thank you for this link.

> > -	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.

==> It's why I suppress the bzero initialisation.

> Regards,
> Didier