Re: [PATCH] mountlist: fix local drive detection on cygwin

[Jim Meyering]

Eric Blake wrote:
> * lib/mountlist.c (ME_REMOTE) [__CYGWIN__]: Provide implementation
> that works for cygwin.
>
> Signed-off-by: Eric Blake <address@hidden>
> ---
>
> Any objections to applying this patch?  I used it when building
> the official cygwin port of coreutils 8.8.  In cygwin's getmntent()
> implementation, all mount points are mapped to a windows-style
> path (but using / instead of \), therefore, even local disks begin
> with a drive letter, and are rejected by the default ME_REMOTE.
>
>  ChangeLog       |    6 ++++++
>  lib/mountlist.c |   24 ++++++++++++++++++++++++
>  2 files changed, 30 insertions(+), 0 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index 0a17782..4074c37 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,9 @@
> +2010-12-28  Eric Blake  <address@hidden>
> +
> +     mountlist: fix local drive detection on cygwin
> +     * lib/mountlist.c (ME_REMOTE) [__CYGWIN__]: Provide implementation
> +     that works for cygwin.
> +
>  2010-12-28  Jim Meyering  <address@hidden>
>
>       regex: don't infloop on persistent failing calloc
> diff --git a/lib/mountlist.c b/lib/mountlist.c
> index 996b71a..ff1e525 100644
> --- a/lib/mountlist.c
> +++ b/lib/mountlist.c
> @@ -156,6 +156,30 @@
>       || strcmp (Fs_type, "ignore") == 0)
>  #endif
>
> +#ifdef __CYGWIN__
> +# include <windows.h>
> +# define ME_REMOTE me_remote
> +/* All cygwin mount points include `:' or start with `//'; so it
> +   requires a native Windows call to determine remote disks.  */
> +static bool
> +me_remote (char const *fs_name, char const *fs_type)
> +{
> +  if (fs_name[0] && fs_name[1] == ':')
> +    {
> +      char const drive[3] = { fs_name[0], ':' };

Hi Eric,
Thanks for doing that.
What do you think about adding an explicit trailing NUL byte there?

         char const drive[3] = { fs_name[0], ':', '\0' };

There is probably some rule about partial aggregate initializers
in C "const" variables that says unspecified elements get 0,
but it's easy to remove all doubt.

> +      switch (GetDriveType (drive))
> +        {
> +        case DRIVE_REMOVABLE:
> +        case DRIVE_FIXED:
> +        case DRIVE_CDROM:
> +        case DRIVE_RAMDISK:
> +          return false;
> +        }
> +    }
> +  return true;
> +}
> +#endif
> +
>  #ifndef ME_REMOTE
>  /* A file system is `remote' if its Fs_name contains a `:'
>     or if (it is of type (smbfs or cifs) and its Fs_name starts with `//').  
> */