[Home]

Summary:ASTERISK-12263: [patch] Union rather than cast for outbuf (translate.h)
Reporter:snuffy (snuffy)Labels:
Date Opened:2008-06-26 10:28:35Date Closed:2011-07-26 14:08:36
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/Portability
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) bug_12932_20080627.diff
( 1) errors.out.txt
Description:This is part of the things i tried to achieve under solaris to fix alginment issues. ( issue ASTERISK-10987 )

This has only had very basic tests done.. alaw/ulaw to gsm etc.
Playback seemed fine.

Probably want to rename some of the unions var names but its a start
Comments:By: Digium Subversion (svnbot) 2008-06-26 11:59:11

Repository: asterisk
Revision: 125386

U   trunk/codecs/codec_a_mu.c
U   trunk/codecs/codec_adpcm.c
U   trunk/codecs/codec_alaw.c
U   trunk/codecs/codec_g722.c
U   trunk/codecs/codec_g726.c
U   trunk/codecs/codec_gsm.c
U   trunk/codecs/codec_ilbc.c
U   trunk/codecs/codec_lpc10.c
U   trunk/codecs/codec_resample.c
U   trunk/codecs/codec_speex.c
U   trunk/codecs/codec_ulaw.c
U   trunk/include/asterisk/translate.h
U   trunk/main/translate.c

------------------------------------------------------------------------
r125386 | tilghman | 2008-06-26 11:59:07 -0500 (Thu, 26 Jun 2008) | 6 lines

Convert casts to unions, to fix alignment issues on Solaris
(closes issue ASTERISK-12263)
Reported by: snuffy
Patches:
      bug_12932_20080627.diff uploaded by snuffy (license 35)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=125386
By: Digium Subversion (svnbot) 2008-06-26 11:59:56

Repository: asterisk
Revision: 125387

_U  branches/1.6.0/

------------------------------------------------------------------------
r125387 | tilghman | 2008-06-26 11:59:55 -0500 (Thu, 26 Jun 2008) | 13 lines

Blocked revisions 125386 via svnmerge

........
r125386 | tilghman | 2008-06-26 12:06:17 -0500 (Thu, 26 Jun 2008) | 6 lines

Convert casts to unions, to fix alignment issues on Solaris
(closes issue ASTERISK-12263)
Reported by: snuffy
Patches:
      bug_12932_20080627.diff uploaded by snuffy (license 35)

........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=125387
By: Tilghman Lesher (tilghman) 2008-06-26 13:45:05

Kevin brought up a dispute on this patch.  Do you have an issue that was fixed with this patch?  Could we see the failure that was caused without this patch?
By: Digium Subversion (svnbot) 2008-06-26 16:00:00

Repository: asterisk
Revision: 125479

_U  team/seanbright/resolve-shadow-warnings/
U   team/seanbright/resolve-shadow-warnings/apps/app_queue.c

------------------------------------------------------------------------
r125479 | seanbright | 2008-06-26 15:59:57 -0500 (Thu, 26 Jun 2008) | 84 lines

Merged revisions 125332-125333,125385-125386,125438,125477 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r125332 | russell | 2008-06-26 11:37:01 -0400 (Thu, 26 Jun 2008) | 5 lines

- add get_max_rate timing API call
- change ast_settimeout() to honor max rate in edge cases of file playback
 (this will make some warning messages go away at the end of playing back
  a file)

................
r125333 | kpfleming | 2008-06-26 11:50:07 -0400 (Thu, 26 Jun 2008) | 13 lines

Merged revisions 125327 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r125327 | kpfleming | 2008-06-26 10:30:33 -0500 (Thu, 26 Jun 2008) | 5 lines

ensure that (whenever possible) if we generate a log message because an ioctl() call to DAHDI/Zaptel failed, that we include the reason it failed by including the stringified error number

(issue AST-80)


........

................
r125385 | oej | 2008-06-26 12:54:22 -0400 (Thu, 26 Jun 2008) | 12 lines


Merged revisions 125384 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r125384 | oej | 2008-06-26 18:32:08 +0200 (Tor, 26 Jun 2008) | 3 lines

Add support for peer realm based auth (a few missing lines, the rest is well documented but never worked)


........

................
r125386 | tilghman | 2008-06-26 13:06:17 -0400 (Thu, 26 Jun 2008) | 6 lines

Convert casts to unions, to fix alignment issues on Solaris
(closes issue ASTERISK-12263)
Reported by: snuffy
Patches:
      bug_12932_20080627.diff uploaded by snuffy (license 35)

................
r125438 | tilghman | 2008-06-26 13:40:25 -0400 (Thu, 26 Jun 2008) | 6 lines

Don't play "your message has been saved" twice.
(closes issue ASTERISK-12228)
Reported by: jaroth
Patches:
      duplicate_saved.patch uploaded by jaroth (license 50)

................
r125477 | mmichelson | 2008-06-26 16:57:41 -0400 (Thu, 26 Jun 2008) | 19 lines

Merged revisions 125476 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r125476 | mmichelson | 2008-06-26 15:56:01 -0500 (Thu, 26 Jun 2008) | 11 lines

Prior to this patch, the "queue show" command used cached
information for realtime queues instead of giving up-to-date
info. Now realtime is queried for the latest and greatest in
queue info.

(closes issue ASTERISK-12195)
Reported by: bcnit
Patches:
     queue_show.patch uploaded by putnopvut (license 60)


........

................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=125479
By: snuffy (snuffy) 2008-06-29 03:48:26

This is the error you will recieve without said patch.

codec_adpcm.c: In function `adpcmtolin_framein':
codec_adpcm.c:233: warning: cast increases required alignment of target type
gmake[1]: *** [codec_adpcm.o] Error 1
gmake[1]: Leaving directory `/global/home/brad/ast-trunk/codecs'
gmake: *** [codecs] Error codec_adpcm.c: In function `adpcmtolin_framein':
codec_adpcm.c:233: warning: cast increases required alignment of target type
gmake[1]: *** [codec_adpcm.o] Error 1
gmake[1]: Leaving directory `/global/home/brad/ast-trunk/codecs'
gmake: *** [codecs] Error 2
By: Kevin P. Fleming (kpfleming) 2008-06-29 12:48:41

Yes, I understand. However, this exposed a larger problem (which was the point of your compiler telling you that), which is that 'outbuf' does not point to an address that is aligned properly for 16-bit access. Just casting it (or using a union to force the cast) won't fix that problem.

I've created a new branch at team/kpfleming/aligner with some new code to fix this problem and ensure that all the elements we allocate *after* the translator private structure are properly aligned. Please give this a try and let me know how it works for you.
By: snuffy (snuffy) 2008-06-30 09:16:14

That does work.
Note: there are still MANY MANY alignment issues to be dealt with.
iax2/dundi/md5/phoneprov/astobj2

If you want to fix all of these would it not be more suited to bug  11485 ?
By: Kevin P. Fleming (kpfleming) 2008-07-03 13:42:01

First, let me say that in fact I would like to fix all the alignment problems that are found on SPARC platforms; there is no reason why we shouldn't be portable to platforms that don't handle unaligned accesses transparently (but slowly) like x86 systems do. For that work, yes, we'll use issue 11485.

However, let's get this one solved first, because it will determine the method we'll use to solve the others in most cases. The first version of the 'aligner' branch, which you reported 'does work', could only have compiled... it had very serious problems and couldn't actually have worked to process calls that required transcoding unless you were very, very lucky :-) I later fixed that branch, but I wasn't happy with that solution.

I have now created a new branch, /team/kpfleming/aligner2. This branch is based on 1.4, not trunk, because I want to fix these issues there first then get them fixed for trunk and 1.6.0 as well. This branch contains a new method of allocating the runtime structure in translate.c, that lets the compiler take care of most of the work, and ensures that we have actually properly aligned each element of the structure, even though we don't know for sure what the structure elements (and their sizes) will be until each time newpvt() gets called.

If you can, I'd very much appreciate some live call testing of this branch, so that we can confirm that this method works as it should. If it does, I'll port these fixes to trunk and 1.6.0, then close this bug and we'll move on to 11485 and try to get the rest of them taken care of.
By: snuffy (snuffy) 2008-07-04 06:47:59

Will be in a position next week to do some tests with that branch ( live calls)
By: snuffy (snuffy) 2008-07-06 23:00:02

Since your branch seems to be based on 1.4
This branch doesn't compile because of sip_addrcmp()
Corydon initially made a union patch see 11485, them later in trunk this was ditched because of astobj2 sip conversion

chan_sip.c:2660: warning: cast increases required alignment of target type


By: snuffy (snuffy) 2008-11-02 19:55:16.000-0600

I have managed to find some time to test compile aligner2 again.

my latest error on compilation (note i'm excluding chan_iax2 and /utils dir in build)

astobj2.c: In function `INTERNAL_OBJ':
astobj2.c:112: warning: cast increases required alignment of target type
gmake[1]: *** [astobj2.o] Error 1
gmake[1]: Leaving directory `/home/support/algin-ast/main'
gmake: *** [main] Error 2
-bash-3.00$ vi main/astobj2.c
By: Digium Subversion (svnbot) 2008-11-03 07:22:00.000-0600

Repository: asterisk
Revision: 153847

U   team/kpfleming/aligner2/main/astobj2.c

------------------------------------------------------------------------
r153847 | kpfleming | 2008-11-03 07:21:59 -0600 (Mon, 03 Nov 2008) | 3 lines

another alignment issue fix related to issue ASTERISK-12263


------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=153847
By: Kevin P. Fleming (kpfleming) 2008-11-03 07:22:33.000-0600

I've committed a fix to aligner2 for that problem; the next time you compile, please use 'make -k' so make will continue past the first error and find as many as we can in one pass. Thanks.
By: snuffy (snuffy) 2008-11-04 15:20:58.000-0600

Ok i've uploaded the gmake -k results..
Note: I got the below error, but fixed it myself by adding a ifndef for _FILE_OFFSET_BITS in the autoconfig.h file.

In file included from /home/support/algin-ast/include/asterisk.h:21,
                from ast_expr2.fl:25:
/home/support/algin-ast/include/asterisk/autoconfig.h:633:1: "_FILE_OFFSET_BITS"                     redefined
In file included from /usr/include/iso/stdarg_c99.h:34,
                from /usr/include/stdarg.h:33,
                from /usr/sfw/lib/gcc/sparc-sun-solaris2.10/3.4.3/include/stdio                    .h:14,
                from ast_expr2f.c:20:
/usr/include/sys/feature_tests.h:188:1: this is the location of the previous def                    inition
By: Tilghman Lesher (tilghman) 2009-02-26 12:53:45.000-0600

Given that related issue ASTERISK-10987 is closed, is this issue moot now?
By: snuffy (snuffy) 2009-02-26 16:23:39.000-0600

Nope.. this one was left open in preference of the other.
By: jmls (jmls) 2010-02-01 07:03:02.000-0600

is there any reason for this to be open still ? It's been nearly a year ;)
By: snuffy (snuffy) 2010-02-01 21:41:12.000-0600

jmls this is a 'placeholder' hopefully we can get around to solving this eventually. :)
By: Leif Madsen (lmadsen) 2011-07-26 14:08:36.101-0500

I'm closing this issue as no progress has been made on it in over 2 years. If someone is interested in moving it forward, just reopen it when you have time. Thanks!