Opened 9 years ago

Closed 8 years ago

#2612 closed enhancement (fixed)

Add support for Win2k

Reported by: chiiph Owned by: chiiph
Priority: Medium Milestone:
Component: Archived/Vidalia Version:
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

This simple fix uses a header from Windows SDK (wspiapi.h) to inline the missing functions in Windows < XP.

The header needs to be in the mingw include directory, and needs a minor correction on the first "_inline" to be "inline".

This patch just adds the header where it needs to be. This was tested only in Win2k.

Child Tickets

Attachments (2)

vidalia.win2k.patch (1.2 KB) - added by chiiph 9 years ago.
vidalia.win2k.2.patch (1.9 KB) - added by chiiph 8 years ago.
New patch with WIN2K define

Download all attachments as: .zip

Change History (11)

Changed 9 years ago by chiiph

Attachment: vidalia.win2k.patch added

comment:1 Changed 9 years ago by chiiph

Status: newneeds_review

comment:2 Changed 9 years ago by edmanm

Hm. I don't think this is really the best way to fix the problem. In particular, it means that everyone who wants to build Vidalia on Windows will have to also download the Windows SDK and remember to shimmy some files around.

A better option would be to update the version of miniupnpc in Vidalia's source tree to at least version 1.5 and add -DNO_GETADDRINFO to src/CMakeLists.txt so that miniupnpc will stop using getaddrinfo() and freeaddrinfo(), which is where the current problem with Windows 2000 originates.

comment:3 Changed 9 years ago by chiiph

The problem is that that define doesn't take into account miniwget.c, which uses another function (I can't remember which right now, but I'll check) that's defined in the header.

To be realistic, most windows users don't build Vidalia, but we can ship it with the actual header, and that shouldn't be of any problem since it's all inline iirc.

comment:4 Changed 9 years ago by edmanm

What function in what header? And ship what header with what inline? If you're talking about shipping wspiapi.h from the Windows SDK in Vidalia's source tree, I think that's a non-starter. The EULA you click through when installing the SDK sure doesn't seem to permit that. If there's something in a miniupnpc file that needs to be tweaked to respect the -DNO_GETADDRINFO flag or avoid some other function that doesn't exist on Win2K, why not just do that?

I agree that most Windows users don't build Vidalia from source, but why should we make life harder for those poor suckers that do? Vidalia is still relatively easy to build for Windows users and I'd sure like to keep it that way if possible.

comment:5 in reply to:  4 ; Changed 8 years ago by chiiph

Replying to edmanm:

What function in what header?

getnameinfo (and may be others that got inlined when including the header file).

And ship what header with what inline? If you're talking about shipping wspiapi.h from the Windows SDK in Vidalia's source tree, I think that's a non-starter. The EULA you click through when installing the SDK sure doesn't seem to permit that.

No, the header file will just be compiled in as a dependency, not shipped with Vidalia's source tree.

If there's something in a miniupnpc file that needs to be tweaked to respect the -DNO_GETADDRINFO flag or avoid some other function that doesn't exist on Win2K, why not just do that?

The macro is GETHOSTBYNAME, but there are a couple of calls that don't use that (and it isn't used in the getnameinfo call either). The code in miniupnpc is really ugly, and the developer already told me: "that needs to be fixed... I think you can make it :)" (literaly). So, I'm not a real fun of hacking the lib to make this work.

I agree that most Windows users don't build Vidalia from source, but why should we make life harder for those poor suckers that do? Vidalia is still relatively easy to build for Windows users and I'd sure like to keep it that way if possible.

I can make a tutorial on how to build Vidalia on Windows, or just add a those steps on a README, and refer the two or three users to that when they complain.

Another possibility is to use a define (disabled by default) "-DWIN2K" or something, that Erinn can use when building the bundles, and the regular user that wants to build it in Windows XP or 7 won't notice a thing.

comment:6 in reply to:  5 ; Changed 8 years ago by edmanm

Replying to chiiph:

Another possibility is to use a define (disabled by default) "-DWIN2K" or something, that Erinn can use when building the bundles, and the regular user that wants to build it in Windows XP or 7 won't notice a thing.

Explain more what this -DWIN2K switch would do. This is possibly a better way to go.

Even if you document that you need to download and install the entire Windows SDK just to build Vidalia on Windows, that doesn't make such a requirement less ridiculous at the moment.

comment:7 in reply to:  6 Changed 8 years ago by chiiph

Replying to edmanm:

Explain more what this -DWIN2K switch would do. This is possibly a better way to go.

It'd be something like:

#ifdef WIN2K

include here whatever we need for win2k to work

#endif

And it'd be only on the couple of files inside miniupnpc that need it.
Here's the updated patch.

Changed 8 years ago by chiiph

Attachment: vidalia.win2k.2.patch added

New patch with WIN2K define

comment:8 Changed 8 years ago by edmanm

Looks agreeable to me. You might also consider adding a note to the INSTALL file mentioning this new option exists.

comment:9 Changed 8 years ago by chiiph

Resolution: fixed
Status: needs_reviewclosed

Right, I added the proper description in the INSTALL file, and the patch's committed.

Note: See TracTickets for help on using tickets.