Re: [PATCH v7 4/5] qemu/osdep: Add excluded fd parameter to qemu_close_all_open_fd()

[Richard Henderson <richard.henderson@linaro.org>]

On 7/31/24 18:48, Clément Léger wrote:
> @@ -823,8 +824,34 @@ static bool qemu_close_all_open_fd_proc(void)
>       dfd = dirfd(dir);
>   
>       for (de = readdir(dir); de; de = readdir(dir)) {
> +        bool close_fd = true;
> +
> +        if (de->d_name[0] == '.') {
> +            continue;
> +        }
>           fd = atoi(de->d_name);
> -        if (fd != dfd) {
> +        if (fd == dfd) {
> +            close_fd = false;
> +            continue;
> +        }

Assignment to close_fd not used here.

> +
> +        for (unsigned int i = skip_start; i < skip_end; i++) {
> +            if (fd < skip[i]) {
> +                /* We are below the next skipped fd, break */
> +                break;
> +            } else if (fd == skip[i]) {
> +                close_fd = false;
> +                /* Restrict the range as we found fds matching start/end */
> +                if (i == skip_start) {
> +                    skip_start++;
> +                } else if (i == skip_end) {
> +                    skip_end--;
> +                }
> +                break;
> +            }
> +        }
> +
> +        if (close_fd) {
>               close(fd);
>           }
>       }
> @@ -833,24 +860,68 @@ static bool qemu_close_all_open_fd_proc(void)
>       return true;
>   }
>   
> -static bool qemu_close_all_open_fd_close_range(void)
> +static bool qemu_close_all_open_fd_close_range(const int *skip,
> +                                               unsigned int nskip,
> +                                               int open_max)
>   {
>   #ifdef CONFIG_CLOSE_RANGE
> -    int r = close_range(0, ~0U, 0);
> -    if (!r) {
> -        /* Success, no need to try other ways. */
> -        return true;
> -    }
> -#endif
> +    int first = 0, last = open_max;

If this were really _SC_OPEN_MAX, this would be off-by-one.

> -static void qemu_close_all_open_fd_fallback(void)
> +static void qemu_close_all_open_fd_fallback(const int *skip, unsigned int nskip,
> +                                            int open_max)
>   {
> -    int open_max = sysconf(_SC_OPEN_MAX), i;
> +    unsigned int cur_skip = 0;
>   
>       /* Fallback */
> -    for (i = 0; i < open_max; i++) {
> +    for (int i = 0; i < open_max; i++) {

Because this isn't really _SC_OPEN_MAX, this *is* off-by-one.

> -void qemu_close_all_open_fd(void)
> +void qemu_close_all_open_fd(const int *skip, unsigned int nskip)
>   {
> -    if (!qemu_close_all_open_fd_close_range() &&
> -        !qemu_close_all_open_fd_proc()) {
> -        qemu_close_all_open_fd_fallback();
> +    int open_max = sysconf(_SC_OPEN_MAX) - 1;

Better to move the -1 into qemu_close_all_open_fd_close_range where it's actually needed.


r~