traviscross/mtr

Hiccups in compiling with MSYS2 on 32bit system.

zcattacz opened this issue · 8 comments

I managed to compile MTR on 32bit Win7 Msys2. But bumped into the following issues.
They are easily hackable, but hopeful they can be handled by the default package configuration.

  1. with auto-detected build type i686-pc-msys the _unix. files got compiled and giving a lot of error. tried to build the package with as under cygwin by specifying --build=i686-pc-cygwin flag.

  2. It started to compile, but stopped with undefined *32 struct error e.g:
    "ICMP_ECHO_REPLY32"
    "IP_OPTION_INFORMATION32"

They are defined in the right place, but I am on 32-bit system and these blocks are enclosed by _WIN64 conditions:

#include <inaddr.h>
typedef struct ip_option_information {
  UCHAR Ttl;
  UCHAR Tos;
  UCHAR Flags;
  UCHAR OptionsSize;
  PUCHAR OptionsData;
} IP_OPTION_INFORMATION,*PIP_OPTION_INFORMATION;
#ifdef _WIN64
typedef struct ip_option_information32 {
  UCHAR Ttl;
  UCHAR Tos;
  UCHAR Flags;
  UCHAR OptionsSize;
  UCHAR *OptionsData;
} IP_OPTION_INFORMATION32,*PIP_OPTION_INFORMATION32;
#endif

Thus tripped down all 32 from struct names like "IP_OPTION_INFORMATION32"

  1. At linking stage, it failed again complaining about - can't find lib cygwin

removed -lcygwin from mtr_packet_LDADD in Makefile.am, tried again, and it worked.

Now MTR compiles fine under msys2. I am not familiar with automake system configuration, though I am comfortable with the compile flags. So these maybe all I can help.

Msys2 is not perfect, but it's certainly an evolving cygwin fork, gradually adopted by more and more people. It would be great to see MTR default configurations can support msys2 right out of the box.

Thanks.

The code you quoted, I can't find that in mtr. Is that a system header?

The mtr code that used IP_OPTION_INFORMATION32 Defines a structure IP_OPTION_INFORMATION and the clears it with sizeof (IP_OPTION_INFORMATION32) That CANNOT be right.

@matt-kimball : IIRC You wrote that, I'm tempted to remove the 32 in IP_OPTION_INFORMATION32 in https://github.com/traviscross/mtr/blob/master/packet/probe_cygwin.c#L233 ... Objections?

@rewolff -- Yes, the sizeof used for the memset looks like an error. I won't have access to my Windows development machine until next week to test any changes, and I did all development using 64-bit Cygwin, but your change does seem like the right thing to do.

Also, I only build the curses interface to mtr for Windows. I've never tried to build the GTK+ interface.

@rewolff yes, you are right the code I quoted is the msys header file specified as #include <w32api/ipexport.h> in packet/probe_cygwin.h. The *32 struct are defined but conditionally to win64 platform. Since I am on 32bit system, this doesn't work. I am not sure what is the proper way to handle this, but since the ones without 32 are defined, I simply striped the 32 suffix.

@zcattacz You showed the definition of IP_OPTION_INFORMATION(32), there is NO difference, if my suspicion that PUCHAR is a pointer to UCHAR so PUCHAR is equivalent to UCHAR *.

Can you show the same definition for the ICMP_ECHO_REPLY32 ?

Where matt doesn't have access to his windows development machine for a while I don't have a windows machine at all.

The attached patch is what I'm guessing that you've done. Correct? Currently uncommitted in my "master" repository.
mtr-windows.patch.txt
So I can't test anything in that code that never activates on my machines.

to separate thread #202

@rewolff
Not need to find a windows machine. looking from here
https://github.com/Alexpux/MSYS2-packages/blob/master/msys2-w32api-headers/PKGBUILD
lead to this
https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/include/ipexport.h
Looks exactly the same file in my version of msys2, though the path is not.

Now we find out why it is a good idea to separate issues into different items in the issue tracker. I've pushed the patch for the -32 thingies. I'd like to mark an issue as solved. most of the discussion here is irrelevant for whatever is left.
zcattacz, can you build separate issues in the tracker for the OTHER things you mention here? Then I can close this one (can you as the issue-starter?)

Hi @rewolff , This ticket is about support on msys2. I think the patch attached is enough to close #194 but not this one yet (e.g. the -lcygwin i.e. cygwin.lib doesn't exist on msys2. (though it actually exist under a different name, and I doesn't seem necessary to link it explicitly) ) I will cut paste the GTK ones into a separate ticket.