For some reason, this E-mail seemed to not go out last time I sent it, so if
you get two, please ignore.
I'm definitely not setting CFLAGS globally anywhere before running make. I'll
check through things and see if I can figure out what's causing the missing
make flags.
static int w_sl_send_reply(struct sip_msg* msg, char* str, char* str2)
{
return sl_send_reply( msg, (unsigned int)(unsigned long)str, str2);
}
The way I am reading this is:
Take the memory address stored in the pointer variable call str (which,
incidentally, that memory address refers to a single byte in memory) and
convert that memory address into an unsigned long (64-bit) number. Then,
convert that unsigned long memory address into an unsigned int (32-bit) number.
Now, if that line were written like this:
return sl_send_reply( msg, (unsigned int)(unsigned long)(*str), str2);
Then, I would agree it is safe, as the (*str) refers to a value that is only
8-bits long. Therefore, converting it to 64-bits and then back to 32-bits
looses no information.
But, that is not the case. The str variable is a pointer variable. Pointer
variables always store 32-bit values on 32-bit architectures and 64-bit values
on 64-bit architectures. The char keyword simply tells the compiler the size,
in bits, of the value stored at the particular address that the pointer
variable stores. On 32-bit architectures, it is essentially equivalent to
saying:
typedef unsigned int ptr;
char ptr str;
Where this would mean that str is a 32-bit memory location that stores an 8-bit
value.
On 64-bit architectures, it would be like saying:
typedef unsigned long ptr;
char ptr str;
Where this would mean that str is a 64-bit memory location that stores an 8-bit
value.
Therefore, the cast of str from a pointer variable to an unsigned long will be
safe on a 64-bit architecture as you do not loose any information. But, the
cast of that 64-bit number to a 32-bit number looses information if the number
is larger than 2^32.
And, as I stated before, I understand that OpenSER is stable for you when you
run it on a 64-bit architecture. In fact, I am running it on 64-bit, as well.
The problem is that unless you have >4GB of memory in the box AND >4GB of
memory in use before you start OpenSER, it will be stable because you will not
run into a case where the memory location is greater than 2^32.
Eliot Gable
Operations Engineer
CCNA, CWNA, CWSP, Network+, Security+
Broadvox, LLC
1228 Euclid Avenue
Suite 390
Cleveland, OH 44115-1800
216-373-4808
> -----Original Message-----
> From: Bogdan-Andrei Iancu [mailto:[EMAIL PROTECTED]
> Sent: Friday, December 22, 2006 1:49 PM
> To: Eliot Gable
> Cc: devel@openser.org
> Subject: Re: [Devel] 64-bit compatability
>
> Hi Eliot,
>
> I guess there is a small problem on you system as non of the compiling
> flags (generated by Makefile) show into the gcc line.
>
> Are you sure you do not have the CFLAGS environment variable set before
> doing make?! You do not need to change anything in the Makefile.defs.
>
> Please double check that. Normally you should get something like:
>
> gcc -g -O9 -funroll-loops -Wcast-align -Wall -minline-all-stringops
> -falign-loops -ftree-vectorize -mtune=opteron -DNAME='"openser"'
> -DVERSION='"1.2.0-dev15-notls"' -DARCH='"x86_64"' -DOS='"linux"'
> -DCOMPILER='"gcc 4.1.2"' -D__CPU_x86_64 -D__OS_linux -D__SMP_yes
> -DCFG_DIR='"/usr/local/etc/openser/"' -DPKG_MALLOC -DSHM_MEM -DSHM_MMAP
> -DUSE_IPV6 -DUSE_MCAST -DUSE_TCP -DDISABLE_NAGLE -DHAVE_RESOLV_RES
> -DSTATISTICS -DF_MALLOC -DFAST_LOCK -DADAPTIVE_WAIT
> -DADAPTIVE_WAIT_LOOPS=1024 -DHAVE_GETHOSTBYNAME2 -DHAVE_UNION_SEMUN
> -DHAVE_SCHED_YIELD -DHAVE_MSG_NOSIGNAL -DHAVE_MSGHDR_MSG_CONTROL
> -DHAVE_ALLOCA_H -DHAVE_TIMEGM -DHAVE_EPOLL -DHAVE_SIGIO_RT -DHAVE_SELECT
> -c action.c -o action.o
>
>
> regarding your alarm on sl_send_reply(), I ensure you that everything is
> ok :). both on 32 and 64 bits - actually we are running openser on 64
> arch for some time with no problem.
>
> I agree with that you are saying, but is not the case as the char* first
> param is used to "transport" and integer. The function must respect a
> generic prototype, but from some time you need to pass an integer; in
> such a case, the int is placed over the pointer by casting - this is
> perfectly safe as sizeof(int)>=sizeof(char*) all the time.
>
> Regards and Merry Christmas,
> Bogdan
>
>
>
> Eliot Gable wrote:
>
> >Using OpenSER-1.1.0-notls:
> >
> >gcc -DNAME='"openser"' -DVERSION='"1.1.0-notls"' -DARCH='"i686"' -
> DOS='"linux"' -DCOMPILER='"gcc 4.1.0"' -D__CPU_i686 -D__OS_linux -
> D__SMP_no -DCFG_DIR='"/usr/local/etc/openser/"' -DPKG_MALLOC -DSHM_MEM -
> DSHM_MMAP -DUSE_IPV6 -DUSE_MCAST -DUSE_TCP -DDISABLE_NAGLE -
> DHAVE_RESOLV_RES -DSTATISTICS -DF_MALLOC -DHAVE_GETHOSTBYNAME2 -
> DHAVE_UNION_SEMUN -DHAVE_SCHED_YIELD -DHAVE_MSG_NOSIGNAL -
> DHAVE_MSGHDR_MSG_CONTROL -DHAVE_ALLOCA_H -DHAVE_TIMEGM -DUSE_SYSV_SEM -
> DHAVE_EPOLL -DHAVE_SIGIO_RT -DHAVE_SELECT -c action.c -o action.o
> >
> >Or, if I turn on extra debugging:
> >
> >gcc -DNAME='"openser"' -DVERSION='"1.1.0-notls"' -DARCH='"i686"' -
> DOS='"linux"' -DCOMPILER='"gcc 4.1.0"' -D__CPU_i686 -D__OS_linux -
> D__SMP_no -DCFG_DIR='"/usr/local/etc/openser/"' -DPKG_MALLOC -DSHM_MEM -
> DSHM_MMAP -DUSE_IPV6 -DUSE_MCAST -DUSE_TCP -DDISABLE_NAGLE -
> DHAVE_RESOLV_RES -DSTATISTICS -DF_MALLOC -DEXTRA_DEBUG -
> DHAVE_GETHOSTBYNAME2 -DHAVE_UNION_SEMUN -DHAVE_SCHED_YIELD -
> DHAVE_MSG_NOSIGNAL -DHAVE_MSGHDR_MSG_CONTROL -DHAVE_ALLOCA_H -DHAVE_TIMEGM
> -DUSE_SYSV_SEM -DHAVE_EPOLL -DHAVE_SIGIO_RT -DHAVE_SELECT -c action.c -o
> action.o
> >
> >If I turn on -Wall by manually adding it to the Makefile.defs here:
> >
> >ifeq ($(mode),debug)
> > DEFS+= -DEXTRA_DEBUG
> >endif
> >
> >Changed to:
> >
> >ifeq ($(mode),debug)
> > DEFS+= -DEXTRA_DEBUG -Wall
> >endif
> >
> >Then I get this line:
> >
> >gcc -DNAME='"openser"' -DVERSION='"1.1.0-notls"' -DARCH='"i686"' -
> DOS='"linux"' -DCOMPILER='"gcc 4.1.0"' -D__CPU_i686 -D__OS_linux -
> D__SMP_no -DCFG_DIR='"/usr/local/etc/openser/"' -DPKG_MALLOC -DSHM_MEM -
> DSHM_MMAP -DUSE_IPV6 -DUSE_MCAST -DUSE_TCP -DDISABLE_NAGLE -
> DHAVE_RESOLV_RES -DSTATISTICS -DF_MALLOC -DEXTRA_DEBUG -Wall -
> DHAVE_GETHOSTBYNAME2 -DHAVE_UNION_SEMUN -DHAVE_SCHED_YIELD -
> DHAVE_MSG_NOSIGNAL -DHAVE_MSGHDR_MSG_CONTROL -DHAVE_ALLOCA_H -DHAVE_TIMEGM
> -DUSE_SYSV_SEM -DHAVE_EPOLL -DHAVE_SIGIO_RT -DHAVE_SELECT -c action.c -o
> action.o
> >
> >
> >But, that doesn't complain about all possible warnings. This does
> >(-W -Wall):
> >
> >gcc -DNAME='"openser"' -DVERSION='"1.1.0-notls"' -DARCH='"i686"' -
> DOS='"linux"' -DCOMPILER='"gcc 4.1.0"' -D__CPU_i686 -D__OS_linux -
> D__SMP_no -DCFG_DIR='"/usr/local/etc/openser/"' -DPKG_MALLOC -DSHM_MEM -
> DSHM_MMAP -DUSE_IPV6 -DUSE_MCAST -DUSE_TCP -DDISABLE_NAGLE -
> DHAVE_RESOLV_RES -DSTATISTICS -DF_MALLOC -DEXTRA_DEBUG -W -Wall -
> DHAVE_GETHOSTBYNAME2 -DHAVE_UNION_SEMUN -DHAVE_SCHED_YIELD -
> DHAVE_MSG_NOSIGNAL -DHAVE_MSGHDR_MSG_CONTROL -DHAVE_ALLOCA_H -DHAVE_TIMEGM
> -DUSE_SYSV_SEM -DHAVE_EPOLL -DHAVE_SIGIO_RT -DHAVE_SELECT -c action.c -o
> action.o
> >In file included from mem/mem.h:54,
> > from lock_alloc.h:50,
> > from locking.h:66,
> > from statistics.h:102,
> > from sr_module.h:42,
> > from action.c:55:
> >mem/f_malloc.h: In function âfm_get_fragsâ:
> >mem/f_malloc.h:162: warning: comparison between signed and unsigned
> >
> >
> >
> >This should be size_t:
> >
> >flags.h:#define MAX_FLAG ((unsigned int)( sizeof(flag_t) * CHAR_BIT - 1
> ))
> >
> >And these:
> >
> >mem/vq_malloc.c: unsigned int real_size;
> >mem/vq_malloc.c:struct vqm_block* vqm_malloc_init(char* address, unsigned
> int size)
> >
> >mem/vq_malloc.c:void* vqm_malloc(struct vqm_block* qm, unsigned int size,
> >mem/vq_malloc.c: char* file, char* func, unsigned int line)
> >mem/vq_malloc.c:void* vqm_malloc(struct vqm_block* qm, unsigned int size)
> >
> >mem/vq_malloc.h:struct vqm_block* vqm_malloc_init(char* address, unsigned
> int size);
> >mem/vq_malloc.h:void* vqm_malloc(struct vqm_block*, unsigned int size,
> char* file, char* func,
> >mem/vq_malloc.h: unsigned int
> line);
> >
> >mem/vq_malloc.h:void* vqm_malloc(struct vqm_block*, unsigned int size);
> >
> >mem/shm_mem.c:inline static void* sh_realloc(void* p, unsigned int size)
> >mem/shm_mem.c:void* _shm_resize( void* p, unsigned int s, const char*
> file, const char* func,
> >mem/shm_mem.c:void* _shm_resize( void* p , unsigned int s)
> >mem/shm_mem.h:inline static void* _shm_malloc(unsigned int size,
> >mem/shm_mem.h:inline static void* _shm_realloc(void *ptr, unsigned int
> size,
> >mem/shm_mem.h:void* _shm_resize(void* ptr, unsigned int size, const char*
> f, const char* fn,
> >mem/shm_mem.h:inline static void* shm_malloc(unsigned int size)
> >mem/shm_mem.h:inline static void* shm_realloc(void *ptr, unsigned int
> size)
> >mem/shm_mem.h:void* _shm_resize(void* ptr, unsigned int size);
> >
> >
> >
> >And what's up with this?:
> >
> >modules/sl/sl.c: return sl_send_reply( msg, (unsigned
> int)(unsigned long)str, str2);
> >
> >Turns out, that is a cast from a char* (which, on 64-bit systems is 64-
> bits or unsigned long) to a 32-bit value:
> >
> >static int w_sl_send_reply(struct sip_msg* msg, char* str, char* str2)
> >{
> > return sl_send_reply( msg, (unsigned int)(unsigned long)str, str2);
> >}
> >
> >If you are on a 64-bit system and that char* points to a location > 4GB
> into memory, then sl_send_reply is in trouble, because it is no longer
> pointing to the same location in memory when inside that function. It is
> now pointing to some random location inside the 4GB envelope.
> >
> >While OpenSER may be compiled on a 64-bit system (which I run a couple,
> as well, and it does compile), it is not 64-bit safe. If you were to run
> this with 32 GB of memory in the machine and use up 8 GB of memory, then
> start OpenSER, it would die spectacularly.
> >
> >I'm not trying to pick a fight or anything -- I just want to make sure
> these obvious errors are caught and resolved. Anything that references a
> length or index to an array, a memory size, or any casts from a pointer to
> some form of integer need to use size_t so that their number of bits
> change with the platform you are compiling the program for.
> >
> >
> >
> >Eliot Gable
> >Operations Engineer
> >CCNA, CWNA, CWSP, Network+, Security+
> >Broadvox, LLC
> >1228 Euclid Avenue
> >Suite 390
> >Cleveland, OH 44115-1800
> >216-373-4808
> >
> >
> >
> >
> >>-----Original Message-----
> >>From: Bogdan-Andrei Iancu [mailto:[EMAIL PROTECTED]
> >>Sent: Friday, December 22, 2006 7:45 AM
> >>To: Eliot Gable
> >>Cc: devel@openser.org
> >>Subject: Re: [Devel] 64-bit compatability
> >>
> >>Hi Eliot,
> >>
> >>the code it checked to get compiled on 64 arch. Actually I'm using a 64
> >>machine for development.
> >>If you get warning, could you specify the openser version and the list
> >>of warnings?
> >>
> >>and, the code is compiled with -Wall... can you post your compiling line
> ?
> >>
> >>Regards,
> >>bogdan
> >>
> >>Eliot Gable wrote:
> >>
> >>
> >>
> >>>Throughout a lot of the code, I have found many instances where memory
> >>>locations are being cast to unsigned int or unsigned long (and even
> >>>sometimes just int) instead of size_t. This can cause a problem on
> >>>64-bit systems due to memory addresses being 64-bit as opposed to 32-
> bit
> >>>like unsigned int or int. If you use size_t, it correctly uses 64-bit
> on
> >>>64-bit systems and 32-bit on 32-bit systems. But that means that in
> >>>str.h, the len parameter must be size_t instead of int to prevent
> >>>warnings about signedness.
> >>>
> >>>Also, is there any reason the code is not being compiled globally with
> >>>-Wall? It catches a lot of these issues for you.
> >>>
> >>>Eliot Gable
> >>>Operations Engineer
> >>>CCNA, CWNA, CWSP, Network+, Security+
> >>>Broadvox, LLC
> >>>1228 Euclid Avenue
> >>>Suite 390
> >>>Cleveland, OH 44115-1800
> >>>216-373-4808
> >>>
> >>>
> >>>
> >>>_______________________________________________
> >>>Devel mailing list
> >>>Devel@openser.org
> >>>http://openser.org/cgi-bin/mailman/listinfo/devel
> >>>
> >>>
> >>>
> >>>
> >>>
> >
> >
> >
> >
_______________________________________________
Devel mailing list
Devel@openser.org
http://openser.org/cgi-bin/mailman/listinfo/devel
