Centralize code for fetching pcap devices. #1434
Conversation
Added memory utilities header for smart pointer utilities.
…Remote)PcapDevices.
| { | ||
| PCPP_LOG_ERROR("Error searching for devices: " << errbuf); | ||
| PCPP_LOG_ERROR(e.what()); |
There was a problem hiding this comment.
recommend to have a meaningful error message here.
There was a problem hiding this comment.
The error messages are actually the same as before. It is just that the DeviceUtil functions have the error messages thrown in a runtime_error as the exception reason and this code catches that and logs the message in the exception.
| pcap_if_t* currInterface = interfaceList; | ||
| while (currInterface != nullptr) | ||
|
|
||
| for (pcap_if_t* currInterface = interfaceList.get(); currInterface != nullptr; currInterface = currInterface->next) |
| } | ||
| catch (const std::exception& e) | ||
| { | ||
| PCPP_LOG_ERROR(e.what()); |
Pcap++/CMakeLists.txt
Outdated
| header/DeviceUtils.h | ||
| header/MemoryUtils.h |
There was a problem hiding this comment.
These 2 new files contain internal utils, so they shouldn't be exposed as part of the API...
There was a problem hiding this comment.
Where do you want me to put them then?
There was a problem hiding this comment.
I'm not a CMake expert, but I think there is a way to decide which header files are "public" (meaning exposed as part of PcapPlusPlus API) and which are "private". This may also affect our doxygen API documentation: https://pcapplusplus.github.io/api-docs/next/ (which currently looks at all the header files in Pcap++/header 😕 )
Maybe @clementperon can help here?
There was a problem hiding this comment.
Well, removed the headers from the public header list. (fb685bf)
Things appear to be building fine, and the only difference is that the headers are missing from the include folder of the install command output.
Don't know if anything has changed in the docs generation. The CI action still shows that the headers are being parsed.
There was a problem hiding this comment.
I think that's ok. For doxygen we probably need to exclude these files in Doxyfile and Doxyfile-ci which are in the web-site repo. Here is how to exclude files:
https://stackoverflow.com/questions/34776315/doxygen-special-command-to-ignore-a-file
Do you want to open a PR for it or prefer I do it?
There was a problem hiding this comment.
Hmm, maybe we can mark the documentation as internal for doxygen in the code?
Something like this: https://stackoverflow.com/a/18443973/11922936
That way the docs won't be generated for the public documentation but it will retain the option for generating internal documentation if we want one in the future.
There was a problem hiding this comment.
Enclosed the internal namespace in a conditional doxygen section (617273e). I think this might remove the namespace's contents from the public documentation.
There was a problem hiding this comment.
Nice!! You can test it locally or merge this PR and I'll test it later. It's not a blocker for merging because the documentation isn't final for a non-releases and we can fix it later if needed
There was a problem hiding this comment.
let's make it in another PR if necessary.
Part of #1431. Implements point 7.