search

yarrick / iodine

Official git repo for iodine dns tunnel
https://code.kryo.se/iodine
ISC License
6.25k stars 507 forks source link

unsigned optimization issues, merge, and Makefile bike shed painting #5

Closed barak closed 10 years ago

barak commented 10 years ago

These are some tweaks to allow make CFLAGS='-O2 -Wall -Wextra -Wno-unused-parameter' to (a) actually issue the appropriate commands, (b) visibly so I can see what's going on, (c) not get too many false positives, (d) not have the optimization do something evil like optimize away x<0 to false when x is unsigned but arguably shouldn't be, and such.

yarrick commented 10 years ago

Thanks, I have merged most of the code. Is the armel build failure still really an issue? Not sure about the makefile changes either.

barak commented 10 years ago

Not sure of the armel build failure; that was a debian autobuild issue, and the debian package is carrying a patch for it. That is really all I know. The maintainer of the debian package might know more.

With the Makefile, I was basically just trying to be more standard. I don't think I added any linux-only changes, although I might have inadvertently used gnu make features that are not part of posix make or bsd make or whatever. Some build systems (cmake?) suppress the actual compiler invocation lines. That is nice ... until one actually needs to see it, at which point it can be frustrating.

I could try to break the Makefile tweaks up into a series of patches which each change only one thing, if you'd like. But the Makefile itself seemed pretty short.

yarrick commented 10 years ago

24f1959bab2de3e17f667bfd7a5d9a207a242f6a should have solved the build failure.

barak commented 10 years ago

Agreed, you are right. I (well, gbp-pq --time-machine import, actually) was too aggressive in forward-porting that patch over yours. Woops.

yarrick commented 10 years ago

BSD make does not like the makefile changes:

$ bmake OS is , arch is cc -pipe -g -g -Wall -pedantic -D -DGITREVISION=\"\" -c tun.c

:0:1: error: macro names must be identifiers tun.c: In function ‘tun_setip’: tun.c:478:2: warning: ISO C90 forbids mixed declarations and code [-Wpedantic] struct in_addr netip; ^ **\* Error code 1 Stop. bmake[1]: stopped in /home/yarrick/code/iodine/src **\* Error code 1 Stop. bmake: stopped in /home/yarrick/code/iodine
barak commented 10 years ago

well I don't like bmake! (will have a look.)

barak commented 10 years ago

From a deeper look at the build issues, I think the "right" thing to do would be to write some autotools-based build scripts: configure.ac, Makefile.am. These would give a more standard and portable build, and would allow things like "./configure --disable-systemd --enable-selinux".

I can whip some up that work for Linux, and should work on *BSD etc. But of course they would require testing, and if BeOS is to enjoy continued support might need a tiny bit of tweaking for that. MS Windows also should work okay, but might need a few tweaks by someone with access to that configuration. (Such tweaks are generally really easy. Like: "uncomment this line and replace the ? by a function provided by the "bind" library on platforms that need to link the "bind" library.)

I'm not sure how to get autotools to work for Android, although apparently Android NDK is supported by autotools so it should be straightforward. (This is something I wouldn't mind learning.)

If you are not in principle opposed to this approach, let me know and I'll give it a shot.