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

[Richard Henderson]

On 7/30/24 22:24, Clément Léger wrote:
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index a6749d9f9b..e7bffaea16 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -808,11 +808,14 @@ int qemu_msync(void *addr, size_t length, int fd)
>       return msync(addr, length, MS_SYNC);
>   }
>   
> -static bool qemu_close_all_open_fd_proc(void)
> +
> +static bool qemu_close_all_open_fd_proc(const int *skip, unsigned int nskip)

Extra whitespace.

>   {
>       struct dirent *de;
>       int fd, dfd;
> +    bool close_fd;
>       DIR *dir;
> +    unsigned int i, skip_start = 0, skip_end = nskip;

You can narrow the scope of close_fd and i.

> @@ -823,8 +826,31 @@ 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) {
> +        close_fd = true;
> +        if (fd == dfd) {
> +            close_fd = false;

continue, which avoids

> +        } else {

the else and the subsequent indentation.

> +            for (i = skip_start; i < skip_end; i++) {

for (unsigned int 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,36 +859,79 @@ 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)

Pass in open_max, so that you don't compute it a second time...

>   {
>   #ifdef CONFIG_CLOSE_RANGE
> -    int r = close_range(0, ~0U, 0);
> -    if (!r) {
> -        /* Success, no need to try other ways. */
> -        return true;
> -    }
> -#endif
> +    int max_fd = sysconf(_SC_OPEN_MAX) - 1;

... here

> -void qemu_close_all_open_fd(void)
> +void qemu_close_all_open_fd(const int *skip, unsigned int nskip)
>   {
>       int open_max = sysconf(_SC_OPEN_MAX);
> +    unsigned int cur_skip = 0;
>       int i;
>   
> -    if (qemu_close_all_open_fd_close_range()) {
> +    assert(skip != NULL || nskip == 0);
> +
> +    if (qemu_close_all_open_fd_close_range(skip, nskip)) {
>           return;
>       }
>   
> -    if (qemu_close_all_open_fd_proc()) {
> +    if (qemu_close_all_open_fd_proc(skip, nskip)) {
>           return;
>       }
>   
>       /* Fallback */
>       for (i = 0; i < open_max; i++) {
> +        if (cur_skip < nskip && i == skip[cur_skip]) {
> +            cur_skip++;
> +            continue;
> +        }
>           close(i);

And pass open_max to qemu_close_all_open_fd_fallback as well.


r~