PcapPlusPlus icon indicating copy to clipboard operation
PcapPlusPlus copied to clipboard

Fix building warning due to LightPcapNg

Open tigercosmos opened this issue 1 year ago • 10 comments

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.

tigercosmos avatar Mar 22 '24 01:03 tigercosmos

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

gyl30 avatar Mar 22 '24 02:03 gyl30

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

tigercosmos avatar Mar 22 '24 03:03 tigercosmos

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?

seladb avatar Mar 22 '24 05:03 seladb

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

gyl30 avatar Mar 22 '24 05:03 gyl30

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.

tigercosmos avatar Mar 22 '24 05:03 tigercosmos

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?

gyl30 avatar Mar 22 '24 07:03 gyl30

yes, sounds good

tigercosmos avatar Mar 22 '24 07:03 tigercosmos

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

seladb avatar Mar 26 '24 06:03 seladb

@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 avatar Mar 26 '24 07:03 tigercosmos

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

tigercosmos avatar May 15 '24 17:05 tigercosmos