Opened 8 years ago

Closed 2 years ago

#4533 closed defect (wontfix)

compat.h: tor_socket_t defined as intptr_t instead native SOCKET (Windows)

Reported by: troll_un Owned by:
Priority: Very Low Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: Windows easy tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Windows (32,64,128,256) has type SOCKET for sockets. Tor could use SOCKET instead decoding OS headers and redefine socket type when Window128 happens.

Child Tickets

Change History (10)

comment:1 Changed 8 years ago by nickm

Milestone: Tor: 0.2.3.x-final
Priority: normalmajor

comment:2 Changed 8 years ago by nickm

So the risky thing here is that SOCKET is defined as u_int on win32 and UINT_PTR on win64. If we make this change, we will have tor_socket_t be unsigned on windows and signed everywhere else. Every place that currently checks for sockets being < 0 will instead have to use the SOCKET_OK macro.

That said, I'm okay doing this on 0.2.3.x if we can seriously audit everything that looks at a socket for correctness.

comment:3 Changed 8 years ago by nickm

Status: newneeds_review

At this point the benefit isn't high enough to do the intptr -> socket transition in 0.2.3.x, and I'm not sure it will be. (If win128 makes SOCKET a 128-bit type, it'll probably make intptr_t 128-bit too, and vice versa. But just in case, checking the type size at compile time might be reasonable.)

Still, there are some more places we should be using the macros we have. See branch "bug4533_part1" in my public repo, which also adds a runtime test

comment:4 Changed 8 years ago by asn

< wanoskarnet> "(unsigned)(s) != INVALID_SOCKET" is never false for win64. SOCKET is unsigned and sizeof(SOCKET) is 8 bytes and INVALID_SOCKET defined as (SOCKET)(-1) so (unsigned)(intptr_t)(-1) never equal (SOCKET)(-1). 00000000FFFFFFFF != FFFFFFFFFFFFFFFF

comment:5 Changed 8 years ago by nickm

Separate bug; fix in bug4533_part2 to be applied to 0.2.2.x. Needs review.

comment:6 Changed 8 years ago by nickm

Milestone: Tor: 0.2.3.x-finalTor: unspecified

Merged those branches. Throwing any intptr->SOCKET transition into unspecified.

comment:7 Changed 7 years ago by nickm

Keywords: tor-client added

comment:8 Changed 7 years ago by nickm

Component: Tor ClientTor

comment:9 Changed 4 years ago by nickm

Priority: HighVery Low
Severity: Normal

Inclined to close this as wontfix.

comment:10 Changed 2 years ago by nickm

Resolution: wontfix
Status: needs_reviewclosed
Note: See TracTickets for help on using tickets.