Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hiccups in compiling with MSYS2 on 32bit system. #199

Open
zcattacz opened this issue Jun 3, 2017 · 8 comments
Open

Hiccups in compiling with MSYS2 on 32bit system. #199

zcattacz opened this issue Jun 3, 2017 · 8 comments

Comments

@zcattacz
Copy link

zcattacz commented Jun 3, 2017

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.

@rewolff
Copy link
Collaborator

rewolff commented Jun 3, 2017

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?

@matt-kimball
Copy link
Contributor

@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.

@zcattacz
Copy link
Author

zcattacz commented Jun 3, 2017

@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.

@rewolff
Copy link
Collaborator

rewolff commented Jun 3, 2017

@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.

@zcattacz
Copy link
Author

zcattacz commented Jun 3, 2017

to separate thread #202

@zcattacz
Copy link
Author

zcattacz commented Jun 3, 2017

@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.

@rewolff
Copy link
Collaborator

rewolff commented Jun 3, 2017

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?)

@zcattacz
Copy link
Author

zcattacz commented Jun 4, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants