[Libguestfs] [PATCH] Recognise cd-rom devices in devsparts.c

[Jim Meyering]

Matthew Booth wrote:
> Also:
> * Un-duplicate device detection code by creating a common mapping function.
> * Add some more comments.
> ---
>  daemon/devsparts.c |  168 ++++++++++++++++++++++++++++++----------------------
>  1 files changed, 96 insertions(+), 72 deletions(-)
>
> diff --git a/daemon/devsparts.c b/daemon/devsparts.c
> index 1970e7d..b89682c 100644
> --- a/daemon/devsparts.c
> +++ b/daemon/devsparts.c
> @@ -29,14 +29,19 @@
>  #include "daemon.h"
>  #include "actions.h"
>
> -char **
> -do_list_devices (void)
> +typedef int (*block_dev_func_t)(const char *dev,
> +                                char ***r, int *size, int *alloc);
> +
> +/* Execute a given function for each discovered block device */
> +static char**
> +foreach_block_device (block_dev_func_t func)
>  {
>    char **r = NULL;
>    int size = 0, alloc = 0;
> +
>    DIR *dir;
>    struct dirent *d;
> -  char buf[256];
> +  int err = 0;
>
>    dir = opendir ("/sys/block");
>    if (!dir) {
> @@ -44,121 +49,140 @@ do_list_devices (void)
>      return NULL;
>    }
>
> +  errno = 0;
>    while ((d = readdir (dir)) != NULL) {
...
>    }
>
> -  if (add_string (&r, &size, &alloc, NULL) == -1) {
> -    closedir (dir);
> +  /* Check readdir didn't fail */
> +  if(0 != errno) {
> +    reply_with_perror ("readdir: /sys/block");
> +    free_stringslen(r, size);
>      return NULL;
>    }

It's nice to detect a failing readdir.
However, that errno=0 assignment belongs inside the loop.
A minimal change would produce this ugliness:

    while ((errno = 0, d = readdir (dir)) != NULL) {
      ...
    }

This is more readable (also moving the decl of "d" into the loop):

    while (true) {
      errno = 0;
      struct dirent *d = readdir (dir);
      if (d != NULL)
        break;
      ...
    }