Comment 1 for bug 1498940

Revision history for this message
Klaus Warnke (k-warnke) wrote :

Hello,

this patch and the patch regarding the generic_static stuff is incomplete.
If the generic option is static, the option_static flag must be copied
around with the other fields too.
I do something like this there:

 if(persist->data.generic.option_static == 1)
 {
  opt_ctxt->data.generic = persist->data.generic;
  rohc_decomp_debug(context, "TCP generic option payload = %u bytes", persist->data.generic.load_len);
 }

instead of copy each struct element explicit. You prefer memcpy to avoid alignment issues.
Yeah, everything works on a Intel box, but if go to ARM strange things happens...
On a SPARC platform you may receive a SIGBUS, which helps.
But I assume, If there is a alignment problem, then there is a problem using
the "->" operator with this struct at all, not only when coping the whole data.
This option_static problem is the same here:

  if(discriminator == 0xff)
  {
   opt_ctxt->data.generic = persist->data.generic;
   rohc_decomp_debug(context, "TCP generic option payload = %u bytes", persist->data.generic.load_len);
  }

If no data received, the ->option_static flag must be copied too.
Or explicit:

                    const size_t opt_load_len = persist->data.generic.load_len;

                    opt_ctxt->data.generic.option_static = 0;/*persist->data.generic.option_static;*/
                    opt_ctxt->data.generic.load_len = opt_load_len;
                    memcpy(opt_ctxt->data.generic.load, persist->data.generic.load, opt_load_len);
                    rohc_decomp_debug(context, "TCP generic option payload = %zu bytes", opt_load_len);

I guess, in the case

  else if(discriminator == 0x00)
  {
...
   opt_ctxt->data.generic.option_static = 0;
   opt_ctxt->data.generic.load_len = opt_load_len;
   memcpy(opt_ctxt->data.generic.load, data + read, opt_load_len);
   read += opt_load_len;
   rohc_decomp_debug(context, "TCP generic option payload = %zu bytes", opt_load_len);

Best,
Klaus Warnke