Skip to content

Really need to include pcap-int.h after system includes #6

Closed
leres wants to merge 1 commit into
the-tcpdump-group:masterfrom
leres:master
Closed

Really need to include pcap-int.h after system includes #6
leres wants to merge 1 commit into
the-tcpdump-group:masterfrom
leres:master

Conversation

@leres
Copy link
Copy Markdown

@leres leres commented May 15, 2020

or else libpcap's portability.h causes things to go off the rails.

On FreeBSD 11.3-RELEASE the conflict is with string.h:

In file included from ./search.c:38:
/usr/include/string.h:92:9: error: expected ')'
size_t   strlcat(char * __restrict, const char * __restrict, size_t);
         ^
/usr/src/contrib/libpcap/portability.h:86:24: note: expanded from macro 'strlcat'
        strncat((x), (y), (z) - strlen((x)) - 1)
                              ^
/usr/include/string.h:92:9: note: to match this '('
/usr/src/contrib/libpcap/portability.h:86:9: note: expanded from macro 'strlcat'
        strncat((x), (y), (z) - strlen((x)) - 1)
               ^
In file included from ./search.c:38:
/usr/include/string.h:99:7: error: conflicting types for 'strncat'
char    *strncat(char * __restrict, const char * __restrict, size_t);
         ^
/usr/include/string.h:92:9: note: previous declaration is here
size_t   strlcat(char * __restrict, const char * __restrict, size_t);
         ^
/usr/src/contrib/libpcap/portability.h:86:2: note: expanded from macro 'strlcat'
        strncat((x), (y), (z) - strlen((x)) - 1)
        ^
2 errors generated.
*** [search.o] Error code 1

make[1]: stopped in /wrkdirs/usr/ports/net/tcpslice/work/tcpslice-2837b72
1 error

make[1]: stopped in /wrkdirs/usr/ports/net/tcpslice/work/tcpslice-2837b72
===> Compilation failed unexpectedly.
Try to set MAKE_JOBS_UNSAFE=yes and rebuild before reporting the failure to
the maintainer.
*** Error code 1

Stop.
make: stopped in /usr/ports/net/tcpslice

@leres
Really need to include pcap-int.h after system includes or else
f1301f1 
libpcap's portability.h goes off the rails.
@guyharris
Copy link
Copy Markdown
Member

guyharris commented May 15, 2020

pcap-int isn't a public header, so it shouldn't include it, nor should it be including portability.h, which is also not a public header.

Instead, given that pcap files are not platform-dependent, it should define the pcap file data structures itself, as per the pcap-savefile man page or the RFC-style pcap file format specification (I plan to ask Van Jacobson and Steve McCanne if they want their names on it).

I'll do that.

guyharris added a commit that referenced this pull request May 15, 2020
@guyharris
search: declare the pcap packet header format ourselves.
b2d9755 
It's not something we need to pick up by pulling in an internal libpcap
header; it is *always* the same - if it's not a 32-bit time-in-seconds,
a 32-bit microseconds-part-of-the-time, a 32-bit captured length, and a
32-bit on-the-network length, it's not a valid pcap file.

This should address GitHub issue #6.
@guyharris
Copy link
Copy Markdown
Member

guyharris commented May 15, 2020

I'll do that.

Done, along with a bunch of other modernizations largely stolen from modernizations done to tcpdump. It builds on my FreeBSD 11.2 VM.

@guyharris guyharris closed this May 15, 2020
@leres
Copy link
Copy Markdown
Author

leres commented May 15, 2020

pcap-int isn't a public header, so it shouldn't include it, nor should it be including portability.h, which is also not a public header.

I don't disagree with you, this was the minimal change that allowed the FreeBSD port to build without patching.

Instead, given that pcap files are not platform-dependent, it should define the pcap file data structures itself, as per the pcap-savefile man page or the RFC-style pcap file format specification (I plan to ask Van Jacobson and Steve McCanne if they want their names on it).

I don't know when dependency on pcap-int.h appeared but I just looked and it is not present in the original Network Research Group rcs repo version of search.c. There are only commits by Vern, Steve, and I ending around 2000. And I don't think we had any 64-bit systems at that point.

Rewriting the code to eliminate the dependency is clearly the best solution.

@C0NSTANTINE110 C0NSTANTINE110 mentioned this pull request Jun 30, 2021
Closed
@infrastation
Copy link
Copy Markdown
Member

infrastation commented Dec 28, 2021

For posterity, there is now a FAQ entry about this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@leres @guyharris @infrastation