[Devel] [ openser-Bugs-1614192 ] Socket str description is malformed on big-endian platforms

SourceForge.net noreply at sourceforge.net
Fri Dec 15 18:31:13 CET 2006


Bugs item #1614192, was opened at 2006-12-12 20:43
Message generated for change (Settings changed) made by bogdan_iancu
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=743020&aid=1614192&group_id=139143

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: core
Group: ver devel
>Status: Closed
>Resolution: Fixed
Priority: 5
Private: No
Submitted By: Ovidiu Sas (osas)
Assigned to: Bogdan (bogdan_iancu)
Summary: Socket str description is malformed on big-endian platforms

Initial Comment:
The way that socket string description is constructed is not compatible with big endian platforms.
Here's a patch that will fix the problem:

# cvs diff -Nu socket_info.h
Index: socket_info.h
===================================================================
RCS file: /cvsroot/openser/sip-server/socket_info.h,v
retrieving revision 1.4
diff -u -r1.4 socket_info.h
--- socket_info.h	13 Dec 2005 13:38:30 -0000	1.4
+++ socket_info.h	12 Dec 2006 18:42:12 -0000
@@ -241,11 +241,6 @@
 
 static inline char* socket2str(struct socket_info *sock, char *s, int* len)
 {
-#define PROTOC2UINT(a, b, c, d) htonl((	(((unsigned int)(a))<<24)+ \
-								(((unsigned int)(b))<<16)+  \
-								(((unsigned int)(c))<<8)+  \
-								((unsigned int)(d)) ) | 0x20202020)
-
 	static char buf[MAX_SOCKET_STR];
 	char *p,*p1;
 
@@ -262,26 +257,31 @@
 
 	switch (sock->proto) {
 		case PROTO_UDP:
-			*((unsigned int*)p) = PROTOC2UINT('u', 'd', 'p', ':');
-			p += 4;
+			*(p++) = 'u';
+			*(p++) = 'd';
+			*(p++) = 'p';
 			break;
 		case PROTO_TCP:
-			*((unsigned int*)p) = PROTOC2UINT('t', 'c', 'p', ':');
-			p += 4;
+			*(p++) = 't';
+			*(p++) = 'c';
+			*(p++) = 'p';
 			break;
 		case PROTO_TLS:
-			*((unsigned int*)p) = PROTOC2UINT('t', 'l', 's', ':');
-			p += 4;
+			*(p++) = 't';
+			*(p++) = 'l';
+			*(p++) = 's';
 			break;
 		case PROTO_SCTP:
-			*((unsigned int*)p) = PROTOC2UINT('s', 'c', 't', 'p');
-			p += 4;
-			*(p++) = ':';
+			*(p++) = 's';
+			*(p++) = 'c';
+			*(p++) = 't';
+			*(p++) = 'p';
 			break;
 		default:
 			LOG(L_CRIT,"BUG:socket2str: unsupported proto %d\n", sock->proto);
 			return 0;
 	}
+	*(p++) = ':';
 	memcpy( p, sock->address_str.s, sock->address_str.len);
 	p += sock->address_str.len;
 	*(p++) = ':';

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

Comment By: Ovidiu Sas (osas)
Date: 2006-12-15 18:56

Message:
Logged In: YES 
user_id=1395524
Originator: YES

> the fact that I was keep asking questions was because I was trying to
> undestand the bug before fixing it

I perfectly understand that :-)


> now, that the mistery is solved, I agree that the patch is the right
fix

glad that we figure it out :-)


Regards,
Ovidiu Sas

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

Comment By: Bogdan (bogdan_iancu)
Date: 2006-12-15 18:43

Message:
Logged In: YES 
user_id=1275325
Originator: NO

Hi Ovidiu,

the fact that I was keep asking questions was because I was trying to
undestand the bug before fixing it, as it looked a bit strange to me
(especially if you said it is big endian issue).

now, that the mistery is solved, I agree that the patch is the right fix
and I will applied. Sorry if it took too long.

Thanks and regards,
bogdan

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

Comment By: Ovidiu Sas (osas)
Date: 2006-12-15 18:23

Message:
Logged In: YES 
user_id=1395524
Originator: YES

Hi Bogdan,

It looks like the code is able to deal with BE (8-bit atomic element) but
not with BE (16-bit atomic element).  With the patch that I provided is
working fine for me.

Now it is up to you if you want to patch the code or live it as is.
This piece of code it is called once for each socket during init.  I don't
know how much speed we gain by this shifting optimization ...

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

Comment By: Bogdan (bogdan_iancu)
Date: 2006-12-15 12:33

Message:
Logged In: YES 
user_id=1275325
Originator: NO

Hi Ovidiu,

the debug output is very interesting and it clearly shows that the problem
is not that the platform is little or big endian. If it would be a bug
regarding this, the debug should look like:
         DEBUG:socket2str: <:pdu192.168.2.20:5070>
but it gets to 
         DEBUG:socket2str: <:>

my guess (beware that I'm not a speciallist in architectures :-/ ) is that
the problem is related to how the memry is alligned  for data types. More
exactly  a pointer to "integer" actually point to the last byte (from left
to right) and not to the first.
normally we have (char *p points to offset 0:
 -3 -2 -1  0  1  2  3  4  5  6  7  8
           u  d  p  :  1  9  2  .
but if the int* points to the last byte ->
 -3 -2 -1  0  1  2  3  4  5  6  7  8
  u  d  p  : \0 \0 \0  1  9  2  .

and when printing, you get the ":\0" null terminated string (as showen in
the debug log)

any comments?!?

regards,
bogdan

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

Comment By: Ovidiu Sas (osas)
Date: 2006-12-15 02:04

Message:
Logged In: YES 
user_id=1395524
Originator: YES

Hi Bogdan,

I will run the test for you.
In the mean time, here's the output for openser built in DEBUG mode from
original 1.1.0 sources:

# openser -m 2 -E -D -dddddddddddddddddddd
 0(640) read 2110394194 from /dev/urandom
 0(640) seeding PRNG with 3276539711
 0(640) test random number 690138349
 0(640) pre_init_tls: Entered
 0(640) loading module /opt/lib/openser/modules/dbtext.so
 0(640) loading module /opt/lib/openser/modules/sl.so
....
 0(640) Listening on 
 0(640) DEBUG:socket2str: <:>
              192.168.2.20 [192.168.2.20]:5070
Listening on 
             udp: 192.168.2.20 [192.168.2.20]:5070
Aliases: 
             udp: 192.168.2.20:5070
             udp: LKG7CAC92:5070

WARNING: no fork mode 
...

see the the output of the DEBUG:socket2str

 0(640) DEBUG:socket2str: <:>

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

Comment By: Bogdan (bogdan_iancu)
Date: 2006-12-14 17:05

Message:
Logged In: YES 
user_id=1275325
Originator: NO

Hi Ovidiu,

I agree that the patch is correct, but actually I fail to see the bug :(.
During some tests on sparc64, nothing wrong came up. Could you please run
on the arm the attached test program and send me the output?

thanks and regards,
Bogdan
File Added: test.c

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

Comment By: Ovidiu Sas (osas)
Date: 2006-12-13 16:42

Message:
Logged In: YES 
user_id=1395524
Originator: YES

Hi Bogdan,

I was compiling on arm big endian and It didn't come up properly for me. 
I noticed when I was trying to use dbtext for usrloc (this is how I noticed
and that's why I submitted the other patch for the output of the fifo
usrloc show command).  With this patch, I was able to use the dbtext.

Regards,
Ovidiu Sas

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

Comment By: Bogdan (bogdan_iancu)
Date: 2006-12-13 16:12

Message:
Logged In: YES 
user_id=1275325
Originator: NO

Ovidiu,

I am not so sure if it a bug. First of all the htonl function is used,
which gives you arch. independency. Secondly, I just tried on a sparc
machine (big endian) and the string was properly generated.

regards,
bogdan

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

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=743020&aid=1614192&group_id=139143



More information about the Devel mailing list