seladb / PcapPlusPlus

PcapPlusPlus is a multiplatform C++ library for capturing, parsing and crafting of network packets. It is designed to be efficient, powerful and easy to use. It provides C++ wrappers for the most popular packet processing engines such as libpcap, Npcap, WinPcap, DPDK, AF_XDP and PF_RING.
https://pcapplusplus.github.io/
The Unlicense
2.63k stars 641 forks source link

Fix building warning due to LightPcapNg #1347

Closed tigercosmos closed 2 weeks ago

tigercosmos commented 3 months ago

build log

[  3%] Building C object 3rdParty/LightPcapNg/CMakeFiles/light_pcapng.dir/LightPcapNg/src/light_io.c.o
/tmp/cirrus-ci-build/3rdParty/LightPcapNg/LightPcapNg/src/light_io.c:91:20: warning: incompatible pointer types assigning to 'FILE *' (aka 'struct __sFILE *') from 'light_file' (aka 'struct light_file_t *') [-Wincompatible-pointer-types]
        pcapng->stream.fd = light_open(file_name, LIGHT_OREAD);
                          ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/cirrus-ci-build/3rdParty/LightPcapNg/LightPcapNg/src/light_io.c:117:17: warning: incompatible pointer types passing 'FILE *' (aka 'struct __sFILE *') to parameter of type 'light_file' (aka 'struct light_file_t *') [-Wincompatible-pointer-types]
        if (light_read(pcapng->stream.fd, &block_type, sizeof(block_type)) == -1 ||
                       ^~~~~~~~~~~~~~~~~
/tmp/cirrus-ci-build/3rdParty/LightPcapNg/LightPcapNg/include/light_platform.h:54:30: note: passing argument to parameter 'fd' here
size_t light_read(light_file fd, void *buf, size_t count);
                             ^
/tmp/cirrus-ci-build/3rdParty/LightPcapNg/LightPcapNg/src/light_io.c:118:15: warning: incompatible pointer types passing 'FILE *' (aka 'struct __sFILE *') to parameter of type 'light_file' (aka 'struct light_file_t *') [-Wincompatible-pointer-types]
                        light_read(pcapng->stream.fd, &block_total_length, sizeof(block_total_length)) == -1) {
                                   ^~~~~~~~~~~~~~~~~
/tmp/cirrus-ci-build/3rdParty/LightPcapNg/LightPcapNg/include/light_platform.h:54:30: note: passing argument to parameter 'fd' here
size_t light_read(light_file fd, void *buf, size_t count);
                             ^
/tmp/cirrus-ci-build/3rdParty/LightPcapNg/LightPcapNg/src/light_io.c:132:17: warning: incompatible pointer types passing 'FILE *' (aka 'struct __sFILE *') to parameter of type 'light_file' (aka 'struct light_file_t *') [-Wincompatible-pointer-types]
        if (light_read(pcapng->stream.fd, &block_data[2], block_total_length - 2 * sizeof(uint32_t)) == -1) {
                       ^~~~~~~~~~~~~~~~~
/tmp/cirrus-ci-build/3rdParty/LightPcapNg/LightPcapNg/include/light_platform.h:54:30: note: passing argument to parameter 'fd' here
size_t light_read(light_file fd, void *buf, size_t count);
                             ^
/tmp/cirrus-ci-build/3rdParty/LightPcapNg/LightPcapNg/src/light_io.c:155:14: warning: incompatible pointer types passing 'FILE *' (aka 'struct __sFILE *') to parameter of type 'light_file' (aka 'struct light_file_t *') [-Wincompatible-pointer-types]
        light_close(pcapng->stream.fd);
                    ^~~~~~~~~~~~~~~~~
/tmp/cirrus-ci-build/3rdParty/LightPcapNg/LightPcapNg/include/light_platform.h:57:28: note: passing argument to parameter 'fd' here
int light_close(light_file fd);
                           ^
5 warnings generated.
gyl30 commented 3 months ago

I noticed that the upstream code and our code are not consistent. Perhaps we need to update?

https://github.com/rvelea/LightPcapNg/blob/33296580096f83a9f17ebe7ea3d2c79977a24471/include/light_platform.h#L52

tigercosmos commented 3 months ago

@gyl30 LightPcapNg in our repo is a forked version with some customized implementation. I did an upgrade in this PR: https://github.com/seladb/PcapPlusPlus/pull/1336 recently. Seems I didn't do it perfectly (the warning parts).

seladb commented 3 months ago

When I added LightPcapNg I didn't want to add it as a git submodule because submodules require the user to specifically fetch them (a simple git clone won't fetch them by default).

My original plan was to update the code once in a while - both fetch from upstream, as well as contribute the changes we make to the original repo. Unfortunately I didn't follow that plan...

Maybe it's time to actually do it? Meaning, contribute the patches we have to the original repo, and then update the code to the latest?

gyl30 commented 3 months ago

Agreed, let's sync the updates upstream so that others can benefit from our updates as well.

tigercosmos commented 3 months ago

Maybe it isn't worth it. The original LightPcapNg has been out of maintenance for a long time (many years with a few commits). In addition, it will take a lot of effort to sync our code back to the original LightPcapNg. I think the proper way is to create a forked repo under PcapPlusPlus (organization) , and move the current code to that forked repo. Later, it will be easier to use git to sync from upstream if upstream has new updates.

gyl30 commented 3 months ago

Using a fork and contributing updates back to the upstream, if the upstream cannot accept our updates in a timely manner, we maintain the fork. Would this be better?

tigercosmos commented 3 months ago

yes, sounds good

seladb commented 3 months ago

I forked it here: https://github.com/PcapPlusPlus/LightPcapNg

I think we can now update our patches to this fork and try to push the to upstream

tigercosmos commented 3 months ago

@seladb I think it's better to open another issue to migrate the code here to the forked repo, so just opened #1348.

This issue should be only for fixing the warning.

tigercosmos commented 1 month ago

as the action started to fail due to the warning, this issue should be resolved ASAP. per https://github.com/seladb/PcapPlusPlus/actions/runs/9047392074