diff mbox series

[v2] os-posix: Expand setrlimit() syscall compatibility

Message ID 20240614210638.5959-1-trentmhuber@gmail.com
State New
Headers show
Series [v2] os-posix: Expand setrlimit() syscall compatibility | expand

Commit Message

Trent Huber June 14, 2024, 9:06 p.m. UTC
Darwin uses a subtly different version of the setrlimit() syscall as
described in the COMPATIBILITY section of the macOS man page. The value
of the rlim_cur member has been adjusted accordingly for Darwin-based
systems.

Signed-off-by: Trent Huber <trentmhuber@gmail.com>
---
The previous version assumed OPEN_MAX was a constant defined on all
POSIX systems--turns out it's only a macOS constant. This version adds
preprocessing conditionals to maintain compatibility with Linux.

 os-posix.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Philippe Mathieu-Daudé June 17, 2024, 7:19 a.m. UTC | #1
Hi Trent,

On 14/6/24 23:06, Trent Huber wrote:
> Darwin uses a subtly different version of the setrlimit() syscall as
> described in the COMPATIBILITY section of the macOS man page. The value
> of the rlim_cur member has been adjusted accordingly for Darwin-based
> systems.
> 
> Signed-off-by: Trent Huber <trentmhuber@gmail.com>
> ---
> The previous version assumed OPEN_MAX was a constant defined on all
> POSIX systems--turns out it's only a macOS constant. This version adds
> preprocessing conditionals to maintain compatibility with Linux.
> 
>   os-posix.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/os-posix.c b/os-posix.c
> index a4284e2c07..43f9a43f3f 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -270,7 +270,11 @@ void os_setup_limits(void)
>           return;
>       }
>   
> +#ifdef CONFIG_DARWIN
> +    nofile.rlim_cur = OPEN_MAX < nofile.rlim_max ? OPEN_MAX : nofile.rlim_max;

Why open-code min()? (The man-page also suggests it).

Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +#else
>       nofile.rlim_cur = nofile.rlim_max;
> +#endif
>   
>       if (setrlimit(RLIMIT_NOFILE, &nofile) < 0) {
>           warn_report("unable to set NOFILE limit: %s", strerror(errno));
Daniel P. Berrangé June 17, 2024, 12:52 p.m. UTC | #2
On Fri, Jun 14, 2024 at 05:06:38PM -0400, Trent Huber wrote:
> Darwin uses a subtly different version of the setrlimit() syscall as
> described in the COMPATIBILITY section of the macOS man page. The value
> of the rlim_cur member has been adjusted accordingly for Darwin-based
> systems.
> 
> Signed-off-by: Trent Huber <trentmhuber@gmail.com>
> ---
> The previous version assumed OPEN_MAX was a constant defined on all
> POSIX systems--turns out it's only a macOS constant. This version adds
> preprocessing conditionals to maintain compatibility with Linux.
> 
>  os-posix.c | 4 ++++
>  1 file changed, 4 insertions(+)>

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
Michael Tokarev June 17, 2024, 1:07 p.m. UTC | #3
17.06.2024 10:19, Philippe Mathieu-Daudé wrote:
> Hi Trent,
> 
> On 14/6/24 23:06, Trent Huber wrote:
>> Darwin uses a subtly different version of the setrlimit() syscall as
>> described in the COMPATIBILITY section of the macOS man page. The value
>> of the rlim_cur member has been adjusted accordingly for Darwin-based
>> systems.
>>
>> Signed-off-by: Trent Huber <trentmhuber@gmail.com>
>> ---
>> The previous version assumed OPEN_MAX was a constant defined on all
>> POSIX systems--turns out it's only a macOS constant. This version adds
>> preprocessing conditionals to maintain compatibility with Linux.
>>
>>   os-posix.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/os-posix.c b/os-posix.c
>> index a4284e2c07..43f9a43f3f 100644
>> --- a/os-posix.c
>> +++ b/os-posix.c
>> @@ -270,7 +270,11 @@ void os_setup_limits(void)
>>           return;
>>       }
>> +#ifdef CONFIG_DARWIN
>> +    nofile.rlim_cur = OPEN_MAX < nofile.rlim_max ? OPEN_MAX : nofile.rlim_max;
> 
> Why open-code min()? (The man-page also suggests it).

I guess it's because stddef.h isn't included there, so min() isn't immediately
available :)

Applied to trivial-patches,

/mjt
Philippe Mathieu-Daudé June 17, 2024, 3:15 p.m. UTC | #4
On 17/6/24 15:07, Michael Tokarev wrote:
> 17.06.2024 10:19, Philippe Mathieu-Daudé wrote:
>> Hi Trent,
>>
>> On 14/6/24 23:06, Trent Huber wrote:
>>> Darwin uses a subtly different version of the setrlimit() syscall as
>>> described in the COMPATIBILITY section of the macOS man page. The value
>>> of the rlim_cur member has been adjusted accordingly for Darwin-based
>>> systems.
>>>
>>> Signed-off-by: Trent Huber <trentmhuber@gmail.com>
>>> ---
>>> The previous version assumed OPEN_MAX was a constant defined on all
>>> POSIX systems--turns out it's only a macOS constant. This version adds
>>> preprocessing conditionals to maintain compatibility with Linux.
>>>
>>>   os-posix.c | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/os-posix.c b/os-posix.c
>>> index a4284e2c07..43f9a43f3f 100644
>>> --- a/os-posix.c
>>> +++ b/os-posix.c
>>> @@ -270,7 +270,11 @@ void os_setup_limits(void)
>>>           return;
>>>       }
>>> +#ifdef CONFIG_DARWIN
>>> +    nofile.rlim_cur = OPEN_MAX < nofile.rlim_max ? OPEN_MAX : 
>>> nofile.rlim_max;
>>
>> Why open-code min()? (The man-page also suggests it).
> 
> I guess it's because stddef.h isn't included there, so min() isn't 
> immediately
> available :)

I see os-posix.c -> "qemu/osdep.h" -> <stddef.h>. Anyway,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> Applied to trivial-patches,

Thanks!
Richard Henderson June 17, 2024, 6:21 p.m. UTC | #5
On 6/17/24 08:15, Philippe Mathieu-Daudé wrote:
> On 17/6/24 15:07, Michael Tokarev wrote:
>> 17.06.2024 10:19, Philippe Mathieu-Daudé wrote:
>>> Hi Trent,
>>>
>>> On 14/6/24 23:06, Trent Huber wrote:
>>>> Darwin uses a subtly different version of the setrlimit() syscall as
>>>> described in the COMPATIBILITY section of the macOS man page. The value
>>>> of the rlim_cur member has been adjusted accordingly for Darwin-based
>>>> systems.
>>>>
>>>> Signed-off-by: Trent Huber <trentmhuber@gmail.com>
>>>> ---
>>>> The previous version assumed OPEN_MAX was a constant defined on all
>>>> POSIX systems--turns out it's only a macOS constant. This version adds
>>>> preprocessing conditionals to maintain compatibility with Linux.
>>>>
>>>>   os-posix.c | 4 ++++
>>>>   1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/os-posix.c b/os-posix.c
>>>> index a4284e2c07..43f9a43f3f 100644
>>>> --- a/os-posix.c
>>>> +++ b/os-posix.c
>>>> @@ -270,7 +270,11 @@ void os_setup_limits(void)
>>>>           return;
>>>>       }
>>>> +#ifdef CONFIG_DARWIN
>>>> +    nofile.rlim_cur = OPEN_MAX < nofile.rlim_max ? OPEN_MAX : nofile.rlim_max;
>>>
>>> Why open-code min()? (The man-page also suggests it).
>>
>> I guess it's because stddef.h isn't included there, so min() isn't immediately
>> available :)
> 
> I see os-posix.c -> "qemu/osdep.h" -> <stddef.h>. Anyway,

We also have MIN in osdep.h.


r~
diff mbox series

Patch

diff --git a/os-posix.c b/os-posix.c
index a4284e2c07..43f9a43f3f 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -270,7 +270,11 @@  void os_setup_limits(void)
         return;
     }
 
+#ifdef CONFIG_DARWIN
+    nofile.rlim_cur = OPEN_MAX < nofile.rlim_max ? OPEN_MAX : nofile.rlim_max;
+#else
     nofile.rlim_cur = nofile.rlim_max;
+#endif
 
     if (setrlimit(RLIMIT_NOFILE, &nofile) < 0) {
         warn_report("unable to set NOFILE limit: %s", strerror(errno));