support: Implement <support/descriptors.h> to track file descriptors

Message ID 87d0qlo5eq.fsf@oldenburg.str.redhat.com
State Committed
Headers

Commit Message

Florian Weimer Dec. 1, 2018, 11:25 a.m. UTC
  * Carlos O'Donell:

> On 11/29/18 11:43 PM, Florian Weimer wrote:
>> I could try to count more precisely how many bytes are actually needed,
>> but that seemed like a waste of mental resources.  Should I use
>> xasprintf instead, to make clear that there is no truncation?
>
> Yes, I think xasprintf solves all the problems.

Like this?

I also enhanced the test to test more conditions in the list merging
code.

Thanks,
Florian

----

2018-12-01  Florian Weimer  <fweimer@redhat.com>

	* support/check.h (support_record_failure_is_failed): Declare.
	* support/descriptors.h: New file.
	* support/support_descriptors.c: Likewise.
	* support/tst-support_descriptors.c: Likewise.
	* support/support_record_failure.c
	(support_record_failure_is_failed): New function.
	* support/Makefile (libsupport-routines): Add support_descriptors.
	(tests): Add tst-support_descriptors.
  

Comments

Florian Weimer Dec. 5, 2018, 1:18 p.m. UTC | #1
* Florian Weimer:

> * Carlos O'Donell:
>
>> On 11/29/18 11:43 PM, Florian Weimer wrote:
>>> I could try to count more precisely how many bytes are actually needed,
>>> but that seemed like a waste of mental resources.  Should I use
>>> xasprintf instead, to make clear that there is no truncation?
>>
>> Yes, I think xasprintf solves all the problems.
>
> Like this?
>
> I also enhanced the test to test more conditions in the list merging
> code.

Any further comments?  I would like to commit this.

Thanks,
Florian
  
Carlos O'Donell Dec. 5, 2018, 3:04 p.m. UTC | #2
On 12/1/18 6:25 AM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>> On 11/29/18 11:43 PM, Florian Weimer wrote:
>>> I could try to count more precisely how many bytes are actually needed,
>>> but that seemed like a waste of mental resources.  Should I use
>>> xasprintf instead, to make clear that there is no truncation?
>>
>> Yes, I think xasprintf solves all the problems.
> 
> Like this?
> 
> I also enhanced the test to test more conditions in the list merging
> code.
> 
> Thanks,
> Florian

OK for master. Thank you!

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ----
> 
> 2018-12-01  Florian Weimer  <fweimer@redhat.com>
> 
> 	* support/check.h (support_record_failure_is_failed): Declare.
> 	* support/descriptors.h: New file.
> 	* support/support_descriptors.c: Likewise.
> 	* support/tst-support_descriptors.c: Likewise.
> 	* support/support_record_failure.c
> 	(support_record_failure_is_failed): New function.
> 	* support/Makefile (libsupport-routines): Add support_descriptors.
> 	(tests): Add tst-support_descriptors.
> 
> diff --git a/support/Makefile b/support/Makefile
> index a2536980d1..93a5143016 100644
> --- a/support/Makefile
> +++ b/support/Makefile
> @@ -46,6 +46,7 @@ libsupport-routines = \
>    support_chroot \
>    support_copy_file_range \
>    support_descriptor_supports_holes \
> +  support_descriptors \

OK.

>    support_enter_mount_namespace \
>    support_enter_network_namespace \
>    support_format_address_family \
> @@ -195,6 +196,7 @@ tests = \
>    tst-support-namespace \
>    tst-support_blob_repeat \
>    tst-support_capture_subprocess \
> +  tst-support_descriptors \

OK.

>    tst-support_format_dns_packet \
>    tst-support_quote_blob \
>    tst-support_quote_string \
> diff --git a/support/check.h b/support/check.h
> index e6765289f2..7ea9a86a9c 100644
> --- a/support/check.h
> +++ b/support/check.h
> @@ -183,6 +183,10 @@ int support_report_failure (int status)
>  /* Internal function used to test the failure recording framework.  */
>  void support_record_failure_reset (void);
>  
> +/* Returns true or false depending on whether there have been test
> +   failures or not.  */
> +int support_record_failure_is_failed (void);

OK.

> +
>  __END_DECLS
>  
>  #endif /* SUPPORT_CHECK_H */
> diff --git a/support/descriptors.h b/support/descriptors.h
> new file mode 100644
> index 0000000000..8ec4cbbdfb
> --- /dev/null
> +++ b/support/descriptors.h
> @@ -0,0 +1,47 @@
> +/* Monitoring file descriptor usage.
> +   Copyright (C) 2018 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef SUPPORT_DESCRIPTORS_H
> +#define SUPPORT_DESCRIPTORS_H
> +
> +#include <stdio.h>
> +
> +/* Opaque pointer, for capturing file descriptor lists.  */
> +struct support_descriptors;
> +
> +/* Record the currently open file descriptors and store them in the
> +   returned list.  Terminate the process if the listing operation
> +   fails.  */
> +struct support_descriptors *support_descriptors_list (void);
> +
> +/* Deallocate the list of descriptors.  */
> +void support_descriptors_free (struct support_descriptors *);
> +
> +/* Write the list of descriptors to STREAM, adding PREFIX to each
> +   line.  */
> +void support_descriptors_dump (struct support_descriptors *,
> +                               const char *prefix, FILE *stream);
> +
> +/* Check for file descriptor leaks and other file descriptor changes:
> +   Compare the current list of descriptors with the passed list.
> +   Record a test failure if there are additional open descriptors,
> +   descriptors have been closed, or if a change in file descriptor can
> +   be detected.  */
> +void support_descriptors_check (struct support_descriptors *);
> +
> +#endif /* SUPPORT_DESCRIPTORS_H */

OK.

> diff --git a/support/support_descriptors.c b/support/support_descriptors.c
> new file mode 100644
> index 0000000000..6e768df93a
> --- /dev/null
> +++ b/support/support_descriptors.c
> @@ -0,0 +1,274 @@
> +/* Monitoring file descriptor usage.
> +   Copyright (C) 2018 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <dirent.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <support/check.h>
> +#include <support/support.h>
> +#include <sys/stat.h>
> +#include <sys/sysmacros.h>
> +#include <xunistd.h>
> +
> +struct descriptor
> +{
> +  int fd;
> +  char *link_target;
> +  dev_t dev;
> +  ino64_t ino;
> +};
> +
> +/* Used with qsort.  */
> +static int
> +descriptor_compare (const void *l, const void *r)
> +{
> +  const struct descriptor *left = l;
> +  const struct descriptor *right = r;
> +  /* Cannot overflow due to limited file descriptor range.  */
> +  return left->fd - right->fd;
> +}
> +
> +#define DYNARRAY_STRUCT descriptor_list
> +#define DYNARRAY_ELEMENT struct descriptor
> +#define DYNARRAY_PREFIX descriptor_list_
> +#define DYNARRAY_ELEMENT_FREE(e) free ((e)->link_target)
> +#define DYNARRAY_INITIAL_SIZE 0
> +#include <malloc/dynarray-skeleton.c>
> +
> +struct support_descriptors
> +{
> +  struct descriptor_list list;
> +};
> +
> +struct support_descriptors *
> +support_descriptors_list (void)
> +{
> +  struct support_descriptors *result = xmalloc (sizeof (*result));
> +  descriptor_list_init (&result->list);
> +
> +  DIR *fds = opendir ("/proc/self/fd");
> +  if (fds == NULL)
> +    FAIL_EXIT1 ("opendir (\"/proc/self/fd\"): %m");
> +
> +  while (true)
> +    {
> +      errno = 0;
> +      struct dirent64 *e = readdir64 (fds);
> +      if (e == NULL)
> +        {
> +          if (errno != 0)
> +            FAIL_EXIT1 ("readdir: %m");
> +          break;
> +        }
> +
> +      if (e->d_name[0] == '.')
> +        continue;
> +
> +      char *endptr;
> +      long int fd = strtol (e->d_name, &endptr, 10);
> +      if (*endptr != '\0' || fd < 0 || fd > INT_MAX)
> +        FAIL_EXIT1 ("readdir: invalid file descriptor name: /proc/self/fd/%s",
> +                    e->d_name);
> +
> +      /* Skip the descriptor which is used to enumerate the
> +         descriptors.  */
> +      if (fd == dirfd (fds))
> +        continue;
> +
> +      char *target;
> +      {
> +        char *path = xasprintf ("/proc/self/fd/%ld", fd);
> +        target = xreadlink (path);
> +        free (path);

OK. This looks great now. With xasprintf, this will be robust and we won't have
to revisit this.

> +      }
> +      struct stat64 st;
> +      if (fstat64 (fd, &st) != 0)
> +        FAIL_EXIT1 ("readdir: fstat64 (%ld) failed: %m", fd);
> +
> +      struct descriptor *item = descriptor_list_emplace (&result->list);
> +      if (item == NULL)
> +        FAIL_EXIT1 ("descriptor_list_emplace: %m");
> +      item->fd = fd;
> +      item->link_target = target;
> +      item->dev = st.st_dev;
> +      item->ino = st.st_ino;
> +    }
> +
> +  closedir (fds);
> +
> +  /* Perform a merge join between descrs and current.  This assumes
> +     that the arrays are sorted by file descriptor.  */
> +
> +  qsort (descriptor_list_begin (&result->list),
> +         descriptor_list_size (&result->list),
> +         sizeof (struct descriptor), descriptor_compare);
> +
> +  return result;
> +}
> +
> +void
> +support_descriptors_free (struct support_descriptors *descrs)
> +{
> +  descriptor_list_free (&descrs->list);
> +  free (descrs);
> +}
> +
> +void
> +support_descriptors_dump (struct support_descriptors *descrs,
> +                          const char *prefix, FILE *fp)
> +{
> +  struct descriptor *end = descriptor_list_end (&descrs->list);
> +  for (struct descriptor *d = descriptor_list_begin (&descrs->list);
> +       d != end; ++d)
> +    {
> +      char *quoted = support_quote_string (d->link_target);
> +      fprintf (fp, "%s%d: target=\"%s\" major=%lld minor=%lld ino=%lld\n",
> +               prefix, d->fd, quoted,
> +               (long long int) major (d->dev),
> +               (long long int) minor (d->dev),
> +               (long long int) d->ino);
> +      free (quoted);
> +    }
> +}
> +
> +static void
> +dump_mismatch (bool *first,
> +               struct support_descriptors *descrs,
> +               struct support_descriptors *current)
> +{
> +  if (*first)
> +    *first = false;
> +  else
> +    return;
> +
> +  puts ("error: Differences found in descriptor set");
> +  puts ("Reference descriptor set:");
> +  support_descriptors_dump (descrs, "  ", stdout);
> +  puts ("Current descriptor set:");
> +  support_descriptors_dump (current, "  ", stdout);
> +  puts ("Differences:");
> +}
> +
> +static void
> +report_closed_descriptor (bool *first,
> +                          struct support_descriptors *descrs,
> +                          struct support_descriptors *current,
> +                          struct descriptor *left)
> +{
> +  support_record_failure ();
> +  dump_mismatch (first, descrs, current);
> +  printf ("error: descriptor %d was closed\n", left->fd);
> +}
> +
> +static void
> +report_opened_descriptor (bool *first,
> +                          struct support_descriptors *descrs,
> +                          struct support_descriptors *current,
> +                          struct descriptor *right)
> +{
> +  support_record_failure ();
> +  dump_mismatch (first, descrs, current);
> +  char *quoted = support_quote_string (right->link_target);
> +  printf ("error: descriptor %d was opened (\"%s\")\n", right->fd, quoted);
> +  free (quoted);
> +}
> +
> +void
> +support_descriptors_check (struct support_descriptors *descrs)
> +{
> +  struct support_descriptors *current = support_descriptors_list ();
> +
> +  /* Perform a merge join between descrs and current.  This assumes
> +     that the arrays are sorted by file descriptor.  */
> +
> +  struct descriptor *left = descriptor_list_begin (&descrs->list);
> +  struct descriptor *left_end = descriptor_list_end (&descrs->list);
> +  struct descriptor *right = descriptor_list_begin (&current->list);
> +  struct descriptor *right_end = descriptor_list_end (&current->list);
> +
> +  bool first = true;
> +  while (left != left_end && right != right_end)
> +    {
> +      if (left->fd == right->fd)
> +        {
> +          if (strcmp (left->link_target, right->link_target) != 0)
> +            {
> +              support_record_failure ();
> +              char *left_quoted = support_quote_string (left->link_target);
> +              char *right_quoted = support_quote_string (right->link_target);
> +              dump_mismatch (&first, descrs, current);
> +              printf ("error: descriptor %d changed from \"%s\" to \"%s\"\n",
> +                      left->fd, left_quoted, right_quoted);
> +              free (left_quoted);
> +              free (right_quoted);
> +            }
> +          if (left->dev != right->dev)
> +            {
> +              support_record_failure ();
> +              dump_mismatch (&first, descrs, current);
> +              printf ("error: descriptor %d changed device"
> +                      " from %lld:%lld to %lld:%lld\n",
> +                      left->fd,
> +                      (long long int) major (left->dev),
> +                      (long long int) minor (left->dev),
> +                      (long long int) major (right->dev),
> +                      (long long int) minor (right->dev));
> +            }
> +          if (left->ino != right->ino)
> +            {
> +              support_record_failure ();
> +              dump_mismatch (&first, descrs, current);
> +              printf ("error: descriptor %d changed ino from %lld to %lld\n",
> +                      left->fd,
> +                      (long long int) left->ino, (long long int) right->ino);
> +            }
> +          ++left;
> +          ++right;
> +        }
> +      else if (left->fd < right->fd)
> +        {
> +          /* Gap on the right.  */
> +          report_closed_descriptor (&first, descrs, current, left);
> +          ++left;
> +        }
> +      else
> +        {
> +          /* Gap on the left.  */
> +          TEST_VERIFY_EXIT (left->fd > right->fd);
> +          report_opened_descriptor (&first, descrs, current, right);
> +          ++right;
> +        }
> +    }

OK.

> +
> +  while (left != left_end)
> +    {
> +      /* Closed descriptors (more descriptors on the left).  */
> +      report_closed_descriptor (&first, descrs, current, left);
> +      ++left;
> +    }
> +
> +  while (right != right_end)
> +    {
> +      /* Opened descriptors (more descriptors on the right).  */
> +      report_opened_descriptor (&first, descrs, current, right);
> +      ++right;
> +    }

OK.

> +
> +  support_descriptors_free (current);
> +}
> diff --git a/support/support_record_failure.c b/support/support_record_failure.c
> index 356798f556..17ab1d80ef 100644
> --- a/support/support_record_failure.c
> +++ b/support/support_record_failure.c
> @@ -104,3 +104,11 @@ support_record_failure_reset (void)
>    __atomic_store_n (&state->failed, 0, __ATOMIC_RELAXED);
>    __atomic_add_fetch (&state->counter, 0, __ATOMIC_RELAXED);
>  }
> +
> +int
> +support_record_failure_is_failed (void)
> +{
> +  /* Relaxed MO is sufficient because we need (blocking) external
> +     synchronization for reliable test error reporting anyway.  */
> +  return __atomic_load_n (&state->failed, __ATOMIC_RELAXED);
> +}

OK. Avoids dependency on internal atomic.h header so support/ remains
compilable outside of glibc.

> diff --git a/support/tst-support_descriptors.c b/support/tst-support_descriptors.c
> new file mode 100644
> index 0000000000..5e9e824bc3
> --- /dev/null
> +++ b/support/tst-support_descriptors.c
> @@ -0,0 +1,198 @@
> +/* Tests for monitoring file descriptor usage.
> +   Copyright (C) 2018 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <fcntl.h>
> +#include <stdbool.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <support/capture_subprocess.h>
> +#include <support/check.h>
> +#include <support/descriptors.h>
> +#include <support/support.h>
> +#include <support/xunistd.h>
> +
> +/* This is the next free descriptor that the subprocess will pick.  */
> +static int free_descriptor;
> +
> +static void
> +subprocess_no_change (void *closure)
> +{
> +  struct support_descriptors *descrs = support_descriptors_list ();
> +  int fd = xopen ("/dev/null", O_WRONLY, 0);
> +  TEST_COMPARE (fd, free_descriptor);
> +  xclose (fd);
> +  support_descriptors_free (descrs);
> +}
> +
> +static void
> +subprocess_closed_descriptor (void *closure)
> +{
> +  int fd = xopen ("/dev/null", O_WRONLY, 0);
> +  TEST_COMPARE (fd, free_descriptor);
> +  struct support_descriptors *descrs = support_descriptors_list ();
> +  xclose (fd);
> +  support_descriptors_check (descrs); /* Will report failure.  */
> +  puts ("EOT");
> +  support_descriptors_free (descrs);
> +}
> +
> +static void
> +subprocess_opened_descriptor (void *closure)
> +{
> +  struct support_descriptors *descrs = support_descriptors_list ();
> +  int fd = xopen ("/dev/null", O_WRONLY, 0);
> +  TEST_COMPARE (fd, free_descriptor);
> +  support_descriptors_check (descrs); /* Will report failure.  */
> +  puts ("EOT");
> +  support_descriptors_free (descrs);
> +}
> +
> +static void
> +subprocess_changed_descriptor (void *closure)
> +{
> +  int fd = xopen ("/dev/null", O_WRONLY, 0);
> +  TEST_COMPARE (fd, free_descriptor);
> +  struct support_descriptors *descrs = support_descriptors_list ();
> +  xclose (fd);
> +  TEST_COMPARE (xopen ("/dev", O_DIRECTORY | O_RDONLY, 0), fd);
> +  support_descriptors_check (descrs); /* Will report failure.  */
> +  puts ("EOT");
> +  support_descriptors_free (descrs);
> +}
> +
> +static void
> +report_subprocess_output (const char *name,
> +                          struct support_capture_subprocess *proc)
> +{
> +  printf ("info: BEGIN %s output\n"
> +          "%s"
> +          "info: END %s output\n",
> +          name, proc->out.buffer, name);
> +}
> +
> +/* Use an explicit flag to preserve failure status across
> +   support_record_failure_reset calls.  */
> +static bool good = true;
> +
> +static void
> +test_run (void)
> +{
> +  struct support_capture_subprocess proc = support_capture_subprocess
> +    (&subprocess_no_change, NULL);
> +  support_capture_subprocess_check (&proc, "subprocess_no_change",
> +                                    0, sc_allow_none);
> +  support_capture_subprocess_free (&proc);
> +
> +  char *expected = xasprintf ("\nDifferences:\n"
> +                              "error: descriptor %d was closed\n"
> +                              "EOT\n",
> +                              free_descriptor);
> +  good = good && !support_record_failure_is_failed ();
> +  proc = support_capture_subprocess (&subprocess_closed_descriptor, NULL);
> +  good = good && support_record_failure_is_failed ();
> +  support_record_failure_reset (); /* Discard the reported error.  */
> +  report_subprocess_output ("subprocess_closed_descriptor", &proc);
> +  TEST_VERIFY (strstr (proc.out.buffer, expected) != NULL);
> +  support_capture_subprocess_check (&proc, "subprocess_closed_descriptor",
> +                                    0, sc_allow_stdout);
> +  support_capture_subprocess_free (&proc);
> +  free (expected);
> +
> +  expected = xasprintf ("\nDifferences:\n"
> +                        "error: descriptor %d was opened (\"/dev/null\")\n"
> +                        "EOT\n",
> +                        free_descriptor);
> +  good = good && !support_record_failure_is_failed ();
> +  proc = support_capture_subprocess (&subprocess_opened_descriptor, NULL);
> +  good = good && support_record_failure_is_failed ();
> +  support_record_failure_reset (); /* Discard the reported error.  */
> +  report_subprocess_output ("subprocess_opened_descriptor", &proc);
> +  TEST_VERIFY (strstr (proc.out.buffer, expected) != NULL);
> +  support_capture_subprocess_check (&proc, "subprocess_opened_descriptor",
> +                                    0, sc_allow_stdout);
> +  support_capture_subprocess_free (&proc);
> +  free (expected);
> +
> +  expected = xasprintf ("\nDifferences:\n"
> +                        "error: descriptor %d changed from \"/dev/null\""
> +                        " to \"/dev\"\n"
> +                        "error: descriptor %d changed ino ",
> +                        free_descriptor, free_descriptor);
> +  good = good && !support_record_failure_is_failed ();
> +  proc = support_capture_subprocess (&subprocess_changed_descriptor, NULL);
> +  good = good && support_record_failure_is_failed ();
> +  support_record_failure_reset (); /* Discard the reported error.  */
> +  report_subprocess_output ("subprocess_changed_descriptor", &proc);
> +  TEST_VERIFY (strstr (proc.out.buffer, expected) != NULL);
> +  support_capture_subprocess_check (&proc, "subprocess_changed_descriptor",
> +                                    0, sc_allow_stdout);
> +  support_capture_subprocess_free (&proc);
> +  free (expected);
> +}
> +
> +static int
> +do_test (void)
> +{
> +  puts ("info: initial descriptor set");
> +  {
> +    struct support_descriptors *descrs = support_descriptors_list ();
> +    support_descriptors_dump (descrs, "info:  ", stdout);
> +    support_descriptors_free (descrs);
> +  }
> +
> +  free_descriptor = xopen ("/dev/null", O_WRONLY, 0);
> +  puts ("info: descriptor set with additional free descriptor");
> +  {
> +    struct support_descriptors *descrs = support_descriptors_list ();
> +    support_descriptors_dump (descrs, "info:  ", stdout);
> +    support_descriptors_free (descrs);
> +  }
> +  TEST_VERIFY (free_descriptor >= 3);
> +  xclose (free_descriptor);
> +
> +  /* Initial test run without a sentinel descriptor.  The presence of
> +     such a descriptor exercises different conditions in the list
> +     comparison in support_descriptors_check.  */
> +  test_run ();

OK.

> +
> +  /* Allocate a sentinel descriptor at the end of the descriptor list,
> +     after free_descriptor.  */
> +  int sentinel_fd;
> +  {
> +    int fd = xopen ("/dev/full", O_WRONLY, 0);
> +    TEST_COMPARE (fd, free_descriptor);
> +    sentinel_fd = dup (fd);
> +    TEST_VERIFY_EXIT (sentinel_fd > fd);
> +    xclose (fd);
> +  }
> +  puts ("info: descriptor set with sentinel descriptor");
> +  {
> +    struct support_descriptors *descrs = support_descriptors_list ();
> +    support_descriptors_dump (descrs, "info:  ", stdout);
> +    support_descriptors_free (descrs);
> +  }
> +
> +  /* Second test run with sentinel descriptor.  */
> +  test_run ();

OK.

> +
> +  xclose (sentinel_fd);
> +
> +  return !good;
> +}
> +
> +#include <support/test-driver.c>
>
  
Florian Weimer Dec. 6, 2018, 2:40 p.m. UTC | #3
* Carlos O'Donell:

> On 12/1/18 6:25 AM, Florian Weimer wrote:
>> * Carlos O'Donell:
>> 
>>> On 11/29/18 11:43 PM, Florian Weimer wrote:
>>>> I could try to count more precisely how many bytes are actually needed,
>>>> but that seemed like a waste of mental resources.  Should I use
>>>> xasprintf instead, to make clear that there is no truncation?
>>>
>>> Yes, I think xasprintf solves all the problems.
>> 
>> Like this?
>> 
>> I also enhanced the test to test more conditions in the list merging
>> code.
>> 
>> Thanks,
>> Florian
>
> OK for master. Thank you!

I had to rename the local struct descriptor to struct procfs_descriptor,
due to a conflict with a mach header.  Pushed with that change.

Thanks,
Florian
  

Patch

diff --git a/support/Makefile b/support/Makefile
index a2536980d1..93a5143016 100644
--- a/support/Makefile
+++ b/support/Makefile
@@ -46,6 +46,7 @@  libsupport-routines = \
   support_chroot \
   support_copy_file_range \
   support_descriptor_supports_holes \
+  support_descriptors \
   support_enter_mount_namespace \
   support_enter_network_namespace \
   support_format_address_family \
@@ -195,6 +196,7 @@  tests = \
   tst-support-namespace \
   tst-support_blob_repeat \
   tst-support_capture_subprocess \
+  tst-support_descriptors \
   tst-support_format_dns_packet \
   tst-support_quote_blob \
   tst-support_quote_string \
diff --git a/support/check.h b/support/check.h
index e6765289f2..7ea9a86a9c 100644
--- a/support/check.h
+++ b/support/check.h
@@ -183,6 +183,10 @@  int support_report_failure (int status)
 /* Internal function used to test the failure recording framework.  */
 void support_record_failure_reset (void);
 
+/* Returns true or false depending on whether there have been test
+   failures or not.  */
+int support_record_failure_is_failed (void);
+
 __END_DECLS
 
 #endif /* SUPPORT_CHECK_H */
diff --git a/support/descriptors.h b/support/descriptors.h
new file mode 100644
index 0000000000..8ec4cbbdfb
--- /dev/null
+++ b/support/descriptors.h
@@ -0,0 +1,47 @@ 
+/* Monitoring file descriptor usage.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef SUPPORT_DESCRIPTORS_H
+#define SUPPORT_DESCRIPTORS_H
+
+#include <stdio.h>
+
+/* Opaque pointer, for capturing file descriptor lists.  */
+struct support_descriptors;
+
+/* Record the currently open file descriptors and store them in the
+   returned list.  Terminate the process if the listing operation
+   fails.  */
+struct support_descriptors *support_descriptors_list (void);
+
+/* Deallocate the list of descriptors.  */
+void support_descriptors_free (struct support_descriptors *);
+
+/* Write the list of descriptors to STREAM, adding PREFIX to each
+   line.  */
+void support_descriptors_dump (struct support_descriptors *,
+                               const char *prefix, FILE *stream);
+
+/* Check for file descriptor leaks and other file descriptor changes:
+   Compare the current list of descriptors with the passed list.
+   Record a test failure if there are additional open descriptors,
+   descriptors have been closed, or if a change in file descriptor can
+   be detected.  */
+void support_descriptors_check (struct support_descriptors *);
+
+#endif /* SUPPORT_DESCRIPTORS_H */
diff --git a/support/support_descriptors.c b/support/support_descriptors.c
new file mode 100644
index 0000000000..6e768df93a
--- /dev/null
+++ b/support/support_descriptors.c
@@ -0,0 +1,274 @@ 
+/* Monitoring file descriptor usage.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <dirent.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <sys/stat.h>
+#include <sys/sysmacros.h>
+#include <xunistd.h>
+
+struct descriptor
+{
+  int fd;
+  char *link_target;
+  dev_t dev;
+  ino64_t ino;
+};
+
+/* Used with qsort.  */
+static int
+descriptor_compare (const void *l, const void *r)
+{
+  const struct descriptor *left = l;
+  const struct descriptor *right = r;
+  /* Cannot overflow due to limited file descriptor range.  */
+  return left->fd - right->fd;
+}
+
+#define DYNARRAY_STRUCT descriptor_list
+#define DYNARRAY_ELEMENT struct descriptor
+#define DYNARRAY_PREFIX descriptor_list_
+#define DYNARRAY_ELEMENT_FREE(e) free ((e)->link_target)
+#define DYNARRAY_INITIAL_SIZE 0
+#include <malloc/dynarray-skeleton.c>
+
+struct support_descriptors
+{
+  struct descriptor_list list;
+};
+
+struct support_descriptors *
+support_descriptors_list (void)
+{
+  struct support_descriptors *result = xmalloc (sizeof (*result));
+  descriptor_list_init (&result->list);
+
+  DIR *fds = opendir ("/proc/self/fd");
+  if (fds == NULL)
+    FAIL_EXIT1 ("opendir (\"/proc/self/fd\"): %m");
+
+  while (true)
+    {
+      errno = 0;
+      struct dirent64 *e = readdir64 (fds);
+      if (e == NULL)
+        {
+          if (errno != 0)
+            FAIL_EXIT1 ("readdir: %m");
+          break;
+        }
+
+      if (e->d_name[0] == '.')
+        continue;
+
+      char *endptr;
+      long int fd = strtol (e->d_name, &endptr, 10);
+      if (*endptr != '\0' || fd < 0 || fd > INT_MAX)
+        FAIL_EXIT1 ("readdir: invalid file descriptor name: /proc/self/fd/%s",
+                    e->d_name);
+
+      /* Skip the descriptor which is used to enumerate the
+         descriptors.  */
+      if (fd == dirfd (fds))
+        continue;
+
+      char *target;
+      {
+        char *path = xasprintf ("/proc/self/fd/%ld", fd);
+        target = xreadlink (path);
+        free (path);
+      }
+      struct stat64 st;
+      if (fstat64 (fd, &st) != 0)
+        FAIL_EXIT1 ("readdir: fstat64 (%ld) failed: %m", fd);
+
+      struct descriptor *item = descriptor_list_emplace (&result->list);
+      if (item == NULL)
+        FAIL_EXIT1 ("descriptor_list_emplace: %m");
+      item->fd = fd;
+      item->link_target = target;
+      item->dev = st.st_dev;
+      item->ino = st.st_ino;
+    }
+
+  closedir (fds);
+
+  /* Perform a merge join between descrs and current.  This assumes
+     that the arrays are sorted by file descriptor.  */
+
+  qsort (descriptor_list_begin (&result->list),
+         descriptor_list_size (&result->list),
+         sizeof (struct descriptor), descriptor_compare);
+
+  return result;
+}
+
+void
+support_descriptors_free (struct support_descriptors *descrs)
+{
+  descriptor_list_free (&descrs->list);
+  free (descrs);
+}
+
+void
+support_descriptors_dump (struct support_descriptors *descrs,
+                          const char *prefix, FILE *fp)
+{
+  struct descriptor *end = descriptor_list_end (&descrs->list);
+  for (struct descriptor *d = descriptor_list_begin (&descrs->list);
+       d != end; ++d)
+    {
+      char *quoted = support_quote_string (d->link_target);
+      fprintf (fp, "%s%d: target=\"%s\" major=%lld minor=%lld ino=%lld\n",
+               prefix, d->fd, quoted,
+               (long long int) major (d->dev),
+               (long long int) minor (d->dev),
+               (long long int) d->ino);
+      free (quoted);
+    }
+}
+
+static void
+dump_mismatch (bool *first,
+               struct support_descriptors *descrs,
+               struct support_descriptors *current)
+{
+  if (*first)
+    *first = false;
+  else
+    return;
+
+  puts ("error: Differences found in descriptor set");
+  puts ("Reference descriptor set:");
+  support_descriptors_dump (descrs, "  ", stdout);
+  puts ("Current descriptor set:");
+  support_descriptors_dump (current, "  ", stdout);
+  puts ("Differences:");
+}
+
+static void
+report_closed_descriptor (bool *first,
+                          struct support_descriptors *descrs,
+                          struct support_descriptors *current,
+                          struct descriptor *left)
+{
+  support_record_failure ();
+  dump_mismatch (first, descrs, current);
+  printf ("error: descriptor %d was closed\n", left->fd);
+}
+
+static void
+report_opened_descriptor (bool *first,
+                          struct support_descriptors *descrs,
+                          struct support_descriptors *current,
+                          struct descriptor *right)
+{
+  support_record_failure ();
+  dump_mismatch (first, descrs, current);
+  char *quoted = support_quote_string (right->link_target);
+  printf ("error: descriptor %d was opened (\"%s\")\n", right->fd, quoted);
+  free (quoted);
+}
+
+void
+support_descriptors_check (struct support_descriptors *descrs)
+{
+  struct support_descriptors *current = support_descriptors_list ();
+
+  /* Perform a merge join between descrs and current.  This assumes
+     that the arrays are sorted by file descriptor.  */
+
+  struct descriptor *left = descriptor_list_begin (&descrs->list);
+  struct descriptor *left_end = descriptor_list_end (&descrs->list);
+  struct descriptor *right = descriptor_list_begin (&current->list);
+  struct descriptor *right_end = descriptor_list_end (&current->list);
+
+  bool first = true;
+  while (left != left_end && right != right_end)
+    {
+      if (left->fd == right->fd)
+        {
+          if (strcmp (left->link_target, right->link_target) != 0)
+            {
+              support_record_failure ();
+              char *left_quoted = support_quote_string (left->link_target);
+              char *right_quoted = support_quote_string (right->link_target);
+              dump_mismatch (&first, descrs, current);
+              printf ("error: descriptor %d changed from \"%s\" to \"%s\"\n",
+                      left->fd, left_quoted, right_quoted);
+              free (left_quoted);
+              free (right_quoted);
+            }
+          if (left->dev != right->dev)
+            {
+              support_record_failure ();
+              dump_mismatch (&first, descrs, current);
+              printf ("error: descriptor %d changed device"
+                      " from %lld:%lld to %lld:%lld\n",
+                      left->fd,
+                      (long long int) major (left->dev),
+                      (long long int) minor (left->dev),
+                      (long long int) major (right->dev),
+                      (long long int) minor (right->dev));
+            }
+          if (left->ino != right->ino)
+            {
+              support_record_failure ();
+              dump_mismatch (&first, descrs, current);
+              printf ("error: descriptor %d changed ino from %lld to %lld\n",
+                      left->fd,
+                      (long long int) left->ino, (long long int) right->ino);
+            }
+          ++left;
+          ++right;
+        }
+      else if (left->fd < right->fd)
+        {
+          /* Gap on the right.  */
+          report_closed_descriptor (&first, descrs, current, left);
+          ++left;
+        }
+      else
+        {
+          /* Gap on the left.  */
+          TEST_VERIFY_EXIT (left->fd > right->fd);
+          report_opened_descriptor (&first, descrs, current, right);
+          ++right;
+        }
+    }
+
+  while (left != left_end)
+    {
+      /* Closed descriptors (more descriptors on the left).  */
+      report_closed_descriptor (&first, descrs, current, left);
+      ++left;
+    }
+
+  while (right != right_end)
+    {
+      /* Opened descriptors (more descriptors on the right).  */
+      report_opened_descriptor (&first, descrs, current, right);
+      ++right;
+    }
+
+  support_descriptors_free (current);
+}
diff --git a/support/support_record_failure.c b/support/support_record_failure.c
index 356798f556..17ab1d80ef 100644
--- a/support/support_record_failure.c
+++ b/support/support_record_failure.c
@@ -104,3 +104,11 @@  support_record_failure_reset (void)
   __atomic_store_n (&state->failed, 0, __ATOMIC_RELAXED);
   __atomic_add_fetch (&state->counter, 0, __ATOMIC_RELAXED);
 }
+
+int
+support_record_failure_is_failed (void)
+{
+  /* Relaxed MO is sufficient because we need (blocking) external
+     synchronization for reliable test error reporting anyway.  */
+  return __atomic_load_n (&state->failed, __ATOMIC_RELAXED);
+}
diff --git a/support/tst-support_descriptors.c b/support/tst-support_descriptors.c
new file mode 100644
index 0000000000..5e9e824bc3
--- /dev/null
+++ b/support/tst-support_descriptors.c
@@ -0,0 +1,198 @@ 
+/* Tests for monitoring file descriptor usage.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <fcntl.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <string.h>
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+#include <support/descriptors.h>
+#include <support/support.h>
+#include <support/xunistd.h>
+
+/* This is the next free descriptor that the subprocess will pick.  */
+static int free_descriptor;
+
+static void
+subprocess_no_change (void *closure)
+{
+  struct support_descriptors *descrs = support_descriptors_list ();
+  int fd = xopen ("/dev/null", O_WRONLY, 0);
+  TEST_COMPARE (fd, free_descriptor);
+  xclose (fd);
+  support_descriptors_free (descrs);
+}
+
+static void
+subprocess_closed_descriptor (void *closure)
+{
+  int fd = xopen ("/dev/null", O_WRONLY, 0);
+  TEST_COMPARE (fd, free_descriptor);
+  struct support_descriptors *descrs = support_descriptors_list ();
+  xclose (fd);
+  support_descriptors_check (descrs); /* Will report failure.  */
+  puts ("EOT");
+  support_descriptors_free (descrs);
+}
+
+static void
+subprocess_opened_descriptor (void *closure)
+{
+  struct support_descriptors *descrs = support_descriptors_list ();
+  int fd = xopen ("/dev/null", O_WRONLY, 0);
+  TEST_COMPARE (fd, free_descriptor);
+  support_descriptors_check (descrs); /* Will report failure.  */
+  puts ("EOT");
+  support_descriptors_free (descrs);
+}
+
+static void
+subprocess_changed_descriptor (void *closure)
+{
+  int fd = xopen ("/dev/null", O_WRONLY, 0);
+  TEST_COMPARE (fd, free_descriptor);
+  struct support_descriptors *descrs = support_descriptors_list ();
+  xclose (fd);
+  TEST_COMPARE (xopen ("/dev", O_DIRECTORY | O_RDONLY, 0), fd);
+  support_descriptors_check (descrs); /* Will report failure.  */
+  puts ("EOT");
+  support_descriptors_free (descrs);
+}
+
+static void
+report_subprocess_output (const char *name,
+                          struct support_capture_subprocess *proc)
+{
+  printf ("info: BEGIN %s output\n"
+          "%s"
+          "info: END %s output\n",
+          name, proc->out.buffer, name);
+}
+
+/* Use an explicit flag to preserve failure status across
+   support_record_failure_reset calls.  */
+static bool good = true;
+
+static void
+test_run (void)
+{
+  struct support_capture_subprocess proc = support_capture_subprocess
+    (&subprocess_no_change, NULL);
+  support_capture_subprocess_check (&proc, "subprocess_no_change",
+                                    0, sc_allow_none);
+  support_capture_subprocess_free (&proc);
+
+  char *expected = xasprintf ("\nDifferences:\n"
+                              "error: descriptor %d was closed\n"
+                              "EOT\n",
+                              free_descriptor);
+  good = good && !support_record_failure_is_failed ();
+  proc = support_capture_subprocess (&subprocess_closed_descriptor, NULL);
+  good = good && support_record_failure_is_failed ();
+  support_record_failure_reset (); /* Discard the reported error.  */
+  report_subprocess_output ("subprocess_closed_descriptor", &proc);
+  TEST_VERIFY (strstr (proc.out.buffer, expected) != NULL);
+  support_capture_subprocess_check (&proc, "subprocess_closed_descriptor",
+                                    0, sc_allow_stdout);
+  support_capture_subprocess_free (&proc);
+  free (expected);
+
+  expected = xasprintf ("\nDifferences:\n"
+                        "error: descriptor %d was opened (\"/dev/null\")\n"
+                        "EOT\n",
+                        free_descriptor);
+  good = good && !support_record_failure_is_failed ();
+  proc = support_capture_subprocess (&subprocess_opened_descriptor, NULL);
+  good = good && support_record_failure_is_failed ();
+  support_record_failure_reset (); /* Discard the reported error.  */
+  report_subprocess_output ("subprocess_opened_descriptor", &proc);
+  TEST_VERIFY (strstr (proc.out.buffer, expected) != NULL);
+  support_capture_subprocess_check (&proc, "subprocess_opened_descriptor",
+                                    0, sc_allow_stdout);
+  support_capture_subprocess_free (&proc);
+  free (expected);
+
+  expected = xasprintf ("\nDifferences:\n"
+                        "error: descriptor %d changed from \"/dev/null\""
+                        " to \"/dev\"\n"
+                        "error: descriptor %d changed ino ",
+                        free_descriptor, free_descriptor);
+  good = good && !support_record_failure_is_failed ();
+  proc = support_capture_subprocess (&subprocess_changed_descriptor, NULL);
+  good = good && support_record_failure_is_failed ();
+  support_record_failure_reset (); /* Discard the reported error.  */
+  report_subprocess_output ("subprocess_changed_descriptor", &proc);
+  TEST_VERIFY (strstr (proc.out.buffer, expected) != NULL);
+  support_capture_subprocess_check (&proc, "subprocess_changed_descriptor",
+                                    0, sc_allow_stdout);
+  support_capture_subprocess_free (&proc);
+  free (expected);
+}
+
+static int
+do_test (void)
+{
+  puts ("info: initial descriptor set");
+  {
+    struct support_descriptors *descrs = support_descriptors_list ();
+    support_descriptors_dump (descrs, "info:  ", stdout);
+    support_descriptors_free (descrs);
+  }
+
+  free_descriptor = xopen ("/dev/null", O_WRONLY, 0);
+  puts ("info: descriptor set with additional free descriptor");
+  {
+    struct support_descriptors *descrs = support_descriptors_list ();
+    support_descriptors_dump (descrs, "info:  ", stdout);
+    support_descriptors_free (descrs);
+  }
+  TEST_VERIFY (free_descriptor >= 3);
+  xclose (free_descriptor);
+
+  /* Initial test run without a sentinel descriptor.  The presence of
+     such a descriptor exercises different conditions in the list
+     comparison in support_descriptors_check.  */
+  test_run ();
+
+  /* Allocate a sentinel descriptor at the end of the descriptor list,
+     after free_descriptor.  */
+  int sentinel_fd;
+  {
+    int fd = xopen ("/dev/full", O_WRONLY, 0);
+    TEST_COMPARE (fd, free_descriptor);
+    sentinel_fd = dup (fd);
+    TEST_VERIFY_EXIT (sentinel_fd > fd);
+    xclose (fd);
+  }
+  puts ("info: descriptor set with sentinel descriptor");
+  {
+    struct support_descriptors *descrs = support_descriptors_list ();
+    support_descriptors_dump (descrs, "info:  ", stdout);
+    support_descriptors_free (descrs);
+  }
+
+  /* Second test run with sentinel descriptor.  */
+  test_run ();
+
+  xclose (sentinel_fd);
+
+  return !good;
+}
+
+#include <support/test-driver.c>