Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix building warning due to LightPcapNg #1347

Closed
tigercosmos opened this issue Mar 22, 2024 · 10 comments
Closed

Fix building warning due to LightPcapNg #1347

tigercosmos opened this issue Mar 22, 2024 · 10 comments

Comments

@tigercosmos
Copy link
Collaborator

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
Copy link
Contributor

gyl30 commented Mar 22, 2024

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
Copy link
Collaborator Author

tigercosmos commented Mar 22, 2024

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

@seladb
Copy link
Owner

seladb commented Mar 22, 2024

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
Copy link
Contributor

gyl30 commented Mar 22, 2024

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

@tigercosmos
Copy link
Collaborator Author

tigercosmos commented Mar 22, 2024

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
Copy link
Contributor

gyl30 commented Mar 22, 2024

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
Copy link
Collaborator Author

yes, sounds good

@seladb
Copy link
Owner

seladb commented Mar 26, 2024

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
Copy link
Collaborator Author

@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
Copy link
Collaborator Author

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

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

No branches or pull requests

3 participants