[FIX] Change inet_ntop to inet_ntoa for Windows XP compatibility - #1165
Conversation
|
CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results:
It seems that not all tests were passed completely. This is an indication that the output of some files is not as expected (but might be according to you). Your PR breaks these cases:
Check the result page for more info. |
|
|
||
| int i; | ||
| char ip[50] = ""; | ||
| char ip[15]; |
There was a problem hiding this comment.
I'd say we are one short here (3*4 for digits + 3 for dots + final zero)
Also it would be good to have that as a constant rather than appear twice as a magic number
There was a problem hiding this comment.
Should I fix the other arrays as well? https://github.com/jshin313/ccextractor/blob/594a83cc4ec76b61ebe2f81dbd6af5c090f0f57b/src/lib_ccx/networking.c#L1096
There was a problem hiding this comment.
If you want to create a PR for this today/tomorrow, yes, please :)
There was a problem hiding this comment.
Should I use the INET_ADDRSTRLEN constant? The <netinet/in.h> POSIX®header file says the constant is 16 bytes. But the Windows version is 22 bytes
// Maximum length of address literals (potentially including a port number)
// generated by any address-to-string conversion routine. This length can
// be used when declaring buffers used with getnameinfo, WSAAddressToString,
// inet_ntoa, etc. We just provide one define, rather than one per api,
// to avoid confusion.
//
// The totals are derived from the following data:
// 15: IPv4 address
// 45: IPv6 address including embedded IPv4 address
// 11: Scope Id
// 2: Brackets around IPv6 address when port is present
// 6: Port (including colon)
// 1: Terminating null byte
//
#define INET_ADDRSTRLEN 22
There was a problem hiding this comment.
Seems like a good idea to me to use a built-in constant :)
There was a problem hiding this comment.
If there is a portable built-in constant it's definitely best to use it.
If there isn't one define your own, but really there's nothing wrong with being generous - a few bytes wasted is much much better than an unexpected buffer overrun. Just add a comment next to the definition so the reader doesn't have to figure out where a magic number comes from.
4 participants
In raising this pull request, I confirm the following (please check boxes):
My familiarity with the project is as follows (check one):
#1116
The change allows the command-line version of ccextractor to be built for Windows XP and run on it as well. The following occurs without the changes because inet_ntop() is not supported on Windows XP.
CCX was built on a Windows 10 machine using Visual Studio 2017 targeted for Windows XP and then run on a Windows XP VM. Visual Studio 2019 doesn't support targeting for Windows XP.