Fix getprotobyname missing on Android preventing use#470
Conversation
getprotobyname is stubbed out on Android causing crash_and_burn to be called. This commit removes the icmp support check when compiling for Android and adds a configuration option to compile without the check. Signed-off-by: Paliak <91493239+Paliak@users.noreply.github.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a configuration option to disable the use of getprotobyname and switches to using hardcoded protocol constants (IPPROTO_ICMP and IPPROTO_ICMPV6) for socket initialization. This change improves compatibility with environments like Android where these lookups may fail. A review comment identifies formatting issues in src/socket6.c, specifically the use of tabs instead of spaces and a minor syntax typo in an if condition.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
|
||
|
|
||
| AC_ARG_ENABLE([getprotobyname], | ||
| AS_HELP_STRING([--disable-getprotobyname], [Disable use of getprotobyname]), |
There was a problem hiding this comment.
Is the --disable-getprotobyname flag really necessary? If not, then an attempt should be made to perform an automatic check.
For example, using AC_CHECK_FUNCS()
There was a problem hiding this comment.
This was the most portable way i could think of implementing this. AC_CHECK_FUNCS will return true on Android as getprotobyname does exist. It just always returns NULL. The only automatic way to check this that i know of would be to use AC_RUN_IFELSE but i decided against that as i feel it would be quite messy (and maybe cause issues with cross compilation). I guess the flag could just be removed altogether as #if !(defined(ANDROID) || defined(__ANDROID__)) would suffice for what this PR is trying to accomplish.
Just thought that it could be a nice to have.
There was a problem hiding this comment.
In that case, it would be better to move the Android detection to configure.ac. We already have the case "${target}" in function, and if other systems use the same approach, it can be extended quite easily.
This keeps the exception in the code itself more flexible.
There was a problem hiding this comment.
I would first check whether getprotobyname is present, and then check whether it needs to be disabled again.
Here's my example:
# Detect Operatingsystem
AC_CANONICAL_TARGET
only_clock_realtime=no
disable_getprotobyname=no
case "${target}" in
*darwin*)
only_clock_realtime=yes
;;
*freebsd*)
only_clock_realtime=yes
;;
*openbsd*)
only_clock_realtime=yes
;;
*android*)
disable_getprotobyname=yes
;;
esac
dnl Checks for libraries.
AC_CHECK_FUNCS([getprotobyname])
AS_IF([test "x$disable_getprotobyname" = "xyes"], [
AC_MSG_NOTICE([Android detected: Disabling getprotobyname])
ac_cv_func_getprotobyname=no
])
if test $ac_cv_func_getprotobyname = yes; then
AC_DEFINE([USE_GETPROTOBYNAME], [1], [Define if getprotobyname is available and should be used.])
fi
Signed-off-by: Paliak <91493239+Paliak@users.noreply.github.com>
Signed-off-by: Paliak <91493239+Paliak@users.noreply.github.com>
…k/fping into fix-getprotobyname-android
Signed-off-by: Paliak <91493239+Paliak@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces conditional compilation to bypass getprotobyname on platforms like Android, falling back to default protocol constants (IPPROTO_ICMP and IPPROTO_ICMPV6). The reviewer suggests a more robust and idiomatic approach using Autoconf's standard feature detection (AC_CHECK_FUNCS([getprotobyname])) to define HAVE_GETPROTOBYNAME automatically, rather than hardcoding platform-specific checks and using a custom USE_GETPROTOBYNAME variable.
|
coverage: 87.992% (-0.008%) from 88.0% — Paliak:fix-getprotobyname-android into schweikert:develop |
3 participants
getprotobynameis stubbed out on Android causing crash_and_burn to be called. This commit removes the ICMP support check when compiling for Android and adds a configuration option to compile withoutgetprotobynamecheck.