Fix header includes and build failures on musl

[ghost]

• include stdint.h in files where its types are used
• getsockopt expects socklen_t instead of int as its fifth argument
• remove includes of headers already included in iperf.h
• sort header includes alphabetically
• fixes build using musl libc #331 and closes Fix build using musl libc #344

Signed-off-by: Moritz Kick [email protected]

[bmah888]

Thanks for the pull request! I need to try to make some time to look at this.

I need to think about the "sort header includes" part...it doesn't seem to be strictly needed and there are some cases (not sure if any of them are applicable to this situation) where the order of header inclusion is actually important, so we don't just want to rearrange them arbitrarily.

Thanks for all the other portability fixes though...again I haven't read all of the code but from the description these sounds like a Good Thing (tm).

[ghost]

Yeah the sort part is just for readability, feel free to remove it if it'd introduce misbehaviour.

[bmah888]

At this point I'm leaning towards fixing the stdint.h stuff and the getsockopt(3) type bug. I don't know how to take part of a pull request, so I think the only way to do this is for someone (you, me, doesn't matter) to make a new commit with just the parts I want. A minimal commit to fix this would only need to touch src/cjson.h and src/t_timer.c. Does that seem OK?

[ghost]

I'll rebase and revert the commits, this should automatically update the PR.

Now the remaining problem is that the struct tcp_info and some pieces of struct tcphdr are behind a _GNU_SOURCE guard, which isn't defined anywhere.

Multiple solutions for that are possible:
a) define it globally somewhere in autoconf
b) define it in every file where <netinet/tcp.h> is included (this would touch six files)
c) define it only in iperf.h and clean up the includes in the files where appropriate

[bmah888]

The plan for revising the pull request sounds great, thanks.

We do define _GNU_SOURCE in src/iperf_api.c. Obviously the effects of that are only visible in iperf_api.c and not any other source files. I suppose that's marginally along the lines of your solution b), although I'm not really wedded to that approach. Is there actually something broken on musl with respect to the way that _GNU_SOURCE is defined today? If so, what is it? Just to point us towards a solution.

(Of the remaining solutions you proposed, I'm not a big fan of c) because that requires us to include iperf.h before including any of the system headers, and that could induce some circular dependencies. a) would probably work because iperf_config.h should be included at the top of *.c files.)

[ghost]

Well yes, everytime netinet/tcp.h gets included without defining _GNU_SOURCE first, you basically don't have the struct tcp_info and end up with something along the line of this:

libtool: compile: gcc -DHAVE_CONFIG_H -I. -g -O2 -Wall -MT iperf_error.lo -MD -MP -MF .deps/iperf_error.Tpo -c iperf_error.c -fPIC -DPIC -o .libs/iperf_error.o In file included from iperf_error.c:33:0: iperf.h:71:21: error: field 'tcpInfo' has incomplete type struct tcp_info tcpInfo; /* getsockopt(TCP_INFO) for Linux, {Free,Net}BSD */

Regarding method c)
You could just define _GNU_SOURCE in iperf.h before including netinet/tcp.h and remove the references to netinet/tcp.h from the files needing it, because they already include iperf.h, which makes its include kind of redundant...

I added another commit with that change for review, the other changes have already been commited separately.

[bmah888]

Looks good to me and thanks for the explanation (as well as your work on the patch!). Merged. I'm going to mess around with issue statuses to make them reflect reality. This change should get merged to the 3.1-STABLE branch, which I intend to do before the next release.