[PATCH 2/3] io: Add closefrom [BZ #10353]

[Florian Weimer]

* Adhemerval Zanella via Libc-alpha:

> diff --git a/NEWS b/NEWS
> index 9e829841f6..221eb4d98e 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -30,6 +30,10 @@ Major new features:
>  
>  * On Linux, the close_range function has been added.
>  
> +* The function closefrom has been adeed.  It closes all file descriptors
> +  greater than given integer.  This function is a GNU extension, although it
> +  also present in other systems.

Typo: “adeed”


> diff --git a/io/closefrom.c b/io/closefrom.c
> new file mode 100644
> index 0000000000..7e8f5a1ce5
> --- /dev/null
> +++ b/io/closefrom.c
> @@ -0,0 +1,34 @@

> +void
> +__closefrom (int lowfd)
> +{
> +  int maxfd = __getdtablesize ();
> +  if (maxfd == -1)
> +    __fortify_fail ("closefrom failed to get the file descriptor table size");
> +
> +  for (int i = 0; i < maxfd; i++)
> +    if (i >= lowfd)
> +      if (__close (i) == -1)
> +        __fortify_fail ("closefrom failed to close a file descriptor");
> +}
> +weak_alias (__closefrom, closefrom)

Is __getdtablesize accurate on Hurd?  Then I guess this part is okay.

__close should probably be __close_nocancel_nostatus.  The error check
is incorrect.

> diff --git a/io/tst-closefrom.c b/io/tst-closefrom.c
> new file mode 100644
> index 0000000000..6eb97cec10
> --- /dev/null
> +++ b/io/tst-closefrom.c

> +  int fds[maximum_fd + 1];

Same issue as the other test?  Contiguous set of descriptors or not?

> +#include <support/test-driver.c>
> diff --git a/posix/unistd.h b/posix/unistd.h
> index 32b8161619..43ddd516aa 100644
> --- a/posix/unistd.h
> +++ b/posix/unistd.h
> @@ -352,6 +352,14 @@ extern __off64_t lseek64 (int __fd, __off64_t __offset, int __whence)
>     __THROW.  */
>  extern int close (int __fd);
>  
> +#ifdef __USE_GNU
> +/* Close all open file descriptors greater than or equal to LOWFD.
> +   Negative LOWFD is clamped to 0.
> +
> +   Similar to close, this functions is a cancellation point.  */
> +extern void closefrom (int __lowfd);
> +#endif

I think this function should not be a cancellation point, so you can add
__THROW.

> +weak_alias (__closefrom, closefrom)
> diff --git a/sysdeps/unix/sysv/linux/closefrom_fallback.c b/sysdeps/unix/sysv/linux/closefrom_fallback.c
> new file mode 100644
> index 0000000000..48815941dd
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/closefrom_fallback.c

> +/* Fallback code: iterates over /proc/self/fd, closing each file descriptor
> +   that fall on the criteria.  */
> +_Bool
> +__closefrom_fallback (int from)
> +{
> +  int dirfd = __open_nocancel (FD_TO_FILENAME_PREFIX, O_RDONLY | O_DIRECTORY,
> +			       0);
> +  if (dirfd == -1)
> +    return false;

You could try to close a few descriptors if the error is ENOENT and try
again.

> +  enum { buffer_size = 1024 };
> +  char buffer[buffer_size];
> +  bool ret = true;

buffer_size is only used once, so I think it's not needed.

> +  while (true)
> +    {
> +      ssize_t ret = __getdents64 (dirfd, buffer, sizeof (buffer));
> +      if (ret == -1)
> +        goto err;
> +      else if (ret == 0)
> +        break;
> +
> +      char *begin = buffer, *end = buffer + ret;
> +      while (begin != end)
> +	{
> +          unsigned short int d_reclen;
> +	  memcpy (&d_reclen, begin + offsetof (struct dirent64, d_reclen),
> +		  sizeof (d_reclen));
> +	  const char *dname = begin + offsetof (struct dirent64, d_name);
> +	  begin += d_reclen;
> +
> +	  if (dname[0] == '.')
> +	    continue;
> +
> +	  int fd = 0;
> +	  for (const char *s = dname; (unsigned int) (*s) - '0' < 10; s++)
> +	    fd = 10 * fd + (*s - '0');
> +
> +	  if (fd == dirfd || fd < from)
> +	    continue;
> +
> +	  if (__close_nocancel (fd) < 0)
> +	    goto err;
> +	}
> +    }
> +
> +  ret = true;
> +err:
> +  __close_nocancel (dirfd);
> +  return ret;
> +}

I still think it's necessary to seek to position zero after exhausting
the current buffer and some files have been closed.

The error check for __close_nocancel seems superfluous.  It's possible
that you get EBADF due to a race with another thread, and that does not
seem sufficient reason to terminate the process?

Please also add a brief documentation to the manual (likewise for
close_range).

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill