|Summary:||ASTERISK-03957: [PATCH] add __attribute__ ((packed)) in q921_xxx structure definitions|
|Date Opened:||2005-04-20 02:14:51||Date Closed:||2005-05-15 11:33:49|
|Environment:||Attachments:||( 0) libpri.diff|
|Description:||incoming q921 frames are not handled correct because the wrong data is accessed. |
After adding __attribute__ ((packed)) to the structures in q921/q931.h everything works.
****** ADDITIONAL INFORMATION ******
My Platform is arm7/uclinux/2.0.39/uclibc, arm-elf- toolchain 2.95.3
|Comments:||By: Paul Cadach (pcadach) 2005-04-20 04:24:01|
As I can see current code isn't portable and needs to be slightly re-worked. __attribute__ ((packed)) will eliminate alignment optimizations.
By: Brian West (bkw918) 2005-04-20 08:54:22
As per the bug guidelines this is minor since you have included a patch.
By: Kevin P. Fleming (kpfleming) 2005-04-20 09:50:18
I suspect there is something wrong with the toolchain on your system. All these structures are defined using the u_int8_t type, which has 8-bit size and 8-bit alignment on every platform. As such, the structures do not need to be 'packed', because they will naturally fall on the same alignment that 'packed' would provide.
By: houdini (houdini) 2005-04-20 10:32:10
The toolchain is a known good 2.95.3 from the uclinux guys, but who knows.
I looked into it a bit deeper and it shows that the i only have to add the packed stuff to the <struct q921_header>.
I'm using bri and therefore the bri patch from http://www.junghanns.net/asterisk/downloads
The problem is that the ui setup message which comes in at the cpe is not understood:
< [ 02 ff 03 08 01 01 05 a1 04 02 88 90 18 01 89 6c 08 00 83 33 38 34 37 30 30 70 07 80 33 38 34 37 32 30 71 0b 80 50 32 38 30 33 38
34 37 32 30 ]
< Unnumbered frame:
< SAPI: 00 C/R: 1 EA: 0
< TEI: 127 EA: 1
< M3: 0 P/F: 0 M2: 0 11: 1 [ ??? ]
< 44 bytes of data
XXX Unnumbered Information not implemented XXX
By: Kevin P. Fleming (kpfleming) 2005-04-20 10:33:52
I have applied this patch to CVS HEAD, in spite of the lack of a disclaimer, since it does not actually have any code changes.
Please get a disclaimer on file before posting any more patches with 'substance', so they can be handled expeditiously. Thanks!
By: Russell Bryant (russell) 2005-05-15 11:33:48
It looks like this is only an issue when using a patched version of libpri, so I'm not going to touch this for 1.0.