On 3/9/24 17:21, Daniel P. Berrangé wrote:
On Tue, Sep 03, 2024 at 05:02:44PM +0200, Philippe Mathieu-Daudé wrote:
On 3/9/24 15:37, Clément Léger wrote:
On 03/09/2024 15:34, Philippe Mathieu-Daudé wrote:
On 3/9/24 09:53, Clément Léger wrote:
On 02/09/2024 21:38, Philippe Mathieu-Daudé wrote:
On 30/8/24 13:57, Clément Léger wrote:
On 30/08/2024 13:31, Michael Tokarev wrote:
30.08.2024 14:14, Clément Léger wrote:
On some systems (MacOS for instance), sysconf(_SC_OPEN_MAX) can
return
-1. In that case we should fallback to using the OPEN_MAX define.
According to "man sysconf", the OPEN_MAX define should be present and
provided by either unistd.h and/or limits.h so include them for that
purpose. For other OSes, just assume a maximum of 1024 files
descriptors
as a fallback.
Fixes: 4ec5ebea078e ("qemu/osdep: Move close_all_open_fds() to oslib-
posix")
Reported-by: Daniel P. Berrangé <berra...@redhat.com>
Signed-off-by: Clément Léger <cle...@rivosinc.com>
Reviewed-by: Michael Tokarev <m...@tls.msk.ru>
@@ -928,6 +933,13 @@ static void
qemu_close_all_open_fd_fallback(const
int *skip, unsigned int nskip,
void qemu_close_all_open_fd(const int *skip, unsigned int nskip)
{
int open_max = sysconf(_SC_OPEN_MAX);
+ if (open_max == -1) {
+#ifdef CONFIG_DARWIN
+ open_max = OPEN_MAX;
Missing errno check.
man sysconf states that:
"On error, -1 is returned and errno is set to indicate the error (for
example, EINVAL, indicating that name is invalid)."
So it seems checking for -1 is enough no ? Or were you thinking about
something else ?
Mine (macOS 14.6) is:
RETURN VALUES
If the call to sysconf() is not successful, -1 is returned and
errno is set appropriately. Otherwise, if the variable is
associated with functionality that is not supported, -1 is
returned and errno is not modified. Otherwise, the current
variable value is returned.
Which seems to imply the same than mine right ? -1 is always returned in
case of error and errno might or not be set. So checking for -1 seems
enough to check an error return.
Yes but we can check for the unsupported case. Something like:
long qemu_sysconf(int name, long unsupported_default)
{
int current_errno = errno;
long retval;
retval = sysconf(name);
if (retval == -1) {
if (errno == current_errno) {
return unsupported_default;
}
perror("sysconf");
return -1;
}
return retval;
}
That looks uncessarily complicated, and IMHO makes it less
portable. eg consider macOS behaviour:
"if the variable is associated with functionality that is
not supported, -1 is returned and errno is not modified"
vs Linux documented behaviour:
"If name corresponds to a maximum or minimum limit, and
that limit is indeterminate, -1 is returned and errno
is not changed."
IMHO we should do what Clément already suggested and set a
default anytime retval == -1, and ignore errno. There is
no user benefit from turning errnos into a fatal error
via perror()
Fine by me.
