265313 - Profile locking busted on little-endian machines

Status: RESOLVED FIXED

(Core Graveyard :: Profile: BackEnd, defect, P1)

Description

[Brendan Eich [:brendan]]

<brendan> argh
<brendan> missing ntohl at
http://lxr.mozilla.org/mozilla/source/profile/dirserviceprovider/src/nsProfileLock.cpp#304
<brendan> perhaps better hoisted out of the loop to after
http://lxr.mozilla.org/mozilla/source/profile/dirserviceprovider/src/nsProfileLock.cpp#270
<shaver> nice

I'd like to fix this for aviary 1.0 and 1.7.x.

/be

Comment 1

[Brendan Eich [:brendan]]

Hmm, something's broken at a more fundamental level:

(gdb)
304                             if (addr != inaddr.s_addr)
(gdb) p inaddr
$13 = {s_addr = 2130706433}
(gdb) p/x $
$14 = {s_addr = 0x7f000001}
(gdb) p/x addr
$15 = 0x100007f

inaddr came from PR_GetHostByName, and prnetdb.h says

** Structures returned by network data base library.  All addresses are
** supplied in host order, and returned in network order (suitable for
** use in system calls).

addr, which is in little-endian (host) byte order, came from inet_addr, whose
man page on FC2 says:

       The  inet_addr()  function  converts  the Internet host address cp from
       numbers-and-dots notation into binary data in network byte  order.   If
       the input is invalid, INADDR_NONE (usually -1) is returned.  This is an
       obsolete interface to inet_aton, described  immediately  above;  it  is
       obsolete because -1 is a valid address (255.255.255.255), and inet_aton
       provides a cleaner way to indicate error return.

So it seems inet_addr is returning a number in host byte order, contrary to its
man page and over a decade of BSD history.  Can someone test this on other
platforms and comments?

/be

Comment 2

[Brendan Eich [:brendan]]

Blaming FC2 at this point.  Welcoming data from others.

/be

Comment 3

[Brendan Eich [:brendan]]

[~/src/phoenix/mozilla/js/src-e4x]$ cat foo.c
#include <stdio.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
 
main() {
    in_addr_t n = inet_addr("127.0.0.1");
    char *p = (char *) &n;
    printf("%#x\n", n);
    printf("%d.%d.%d.%d\n", p[0], p[1], p[2], p[3]);
    return 0;
}
[~/src/phoenix/mozilla/js/src-e4x]$ cc -o foo foo.c
[~/src/phoenix/mozilla/js/src-e4x]$ ./foo
0x100007f
127.0.0.1

Oops, I misread the two numbers and blamed the wrong party.  We want 127 on the
lowest address, which for x86 is the LSB.  So it seems PR_GetHostByName is
screwing up.  More in a bit.

/be

Comment 4

[Brendan Eich [:brendan]]

Attachment: obvious fix

Duh, I see the problem.  Sorry for blaming FC2 (I'm alas conditioned by it to
do so :-/).

/be

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 162756 [details] [diff] [review]
obvious fix

I put this fix into a version that had a bunch of whitespace cleanup.  If you
insist I'll extract the minimal fix, but cvs diff -w does not lie, and the
modeline must be upheld! ;-)

/be

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

it looks like the analysis requested by brendan in bug 151188 comment 8 has been
performed :-) this bug does seems to be a duplicate of that one (per bug 151188
comment 0, various other comments, and the duplicates). oh well.

Comment 7

[Brendan Eich [:brendan]]

Comment on attachment 162756 [details] [diff] [review]
obvious fix

Telepathic sr=shaver.  a=me for branches.

/be

Comment 8

[Brendan Eich [:brendan]]

Fixed everywhere.

/be