[gvfs/nfs] Address second set of review comments

[Ross Lagerwall <rossl src gnome org>]

commit d518324bd22da53f18389978fdc7211c40be8e67
Author: Ross Lagerwall <rosslagerwall gmail com>
Date:   Sun Nov 23 21:39:29 2014 +0000

    Address second set of review comments

 daemon/gvfsbackendnfs.c |  297 ++++++++++++++++++++++-------------------------
 daemon/gvfsbackendnfs.h |    7 +-
 2 files changed, 141 insertions(+), 163 deletions(-)
---
diff --git a/daemon/gvfsbackendnfs.c b/daemon/gvfsbackendnfs.c
index 761014f..46012f0 100644
--- a/daemon/gvfsbackendnfs.c
+++ b/daemon/gvfsbackendnfs.c
@@ -1,5 +1,5 @@
 /* GIO - GLib Input, Output and Streaming Library
- * 
+ *
  * Copyright (C) 2014 Ross Lagerwall
  *
  * This library is free software; you can redistribute it and/or
@@ -54,6 +54,7 @@
 #include "gvfsjobmove.h"
 #include "gvfsdaemonprotocol.h"
 #include "gvfsdaemonutils.h"
+#include "gvfsutils.h"
 
 #include <nfsc/libnfs.h>
 #include <nfsc/libnfs-raw-nfs.h>
@@ -231,14 +232,14 @@ do_mount (GVfsBackend *backend,
    * against the latter. */
   for (ptr = export_list; ptr; ptr = ptr->ex_next)
     {
-      if (strstr (mount_spec->mount_prefix, ptr->ex_dir) == mount_spec->mount_prefix)
+      if (g_str_has_prefix (mount_spec->mount_prefix, ptr->ex_dir))
         {
           int this_exportlen = strlen (ptr->ex_dir);
 
           if (pathlen > this_exportlen)
             {
               char *s = mount_spec->mount_prefix + this_exportlen;
-              if ((*s == '/' || *s == '\0') && this_exportlen > exportlen)
+              if (*s == '/' && this_exportlen > exportlen)
                 {
                   export = ptr->ex_dir;
                   exportlen = this_exportlen;
@@ -268,7 +269,16 @@ do_mount (GVfsBackend *backend,
   err = nfs_mount (op_backend->ctx, host, export);
   if (err)
     {
-      g_vfs_job_failed_from_errno (G_VFS_JOB (job), -err);
+      if (err == -EACCES)
+        {
+          g_vfs_job_failed_literal (G_VFS_JOB (job),
+                                    G_IO_ERROR, G_IO_ERROR_PERMISSION_DENIED,
+                                    _("Permission denied: Perhaps this host is disallowed or a privileged 
port is needed"));
+        }
+      else
+        {
+          g_vfs_job_failed_from_errno (G_VFS_JOB (job), -err);
+        }
       return;
     }
 
@@ -480,27 +490,12 @@ set_name_info (GFileInfo *info,
   if (basename[strlen (basename) -1] == '~')
     g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_STANDARD_IS_BACKUP, TRUE);
 
-  /* We use the same setting as for local files. */
-  if (g_file_attribute_matcher_matches (matcher,
-                                        G_FILE_ATTRIBUTE_STANDARD_DISPLAY_NAME))
-    {
-      char *display_name = g_filename_display_name (basename);
-
-      if (strstr (display_name, "\357\277\275") != NULL)
-        {
-          char *p = display_name;
-          display_name = g_strconcat (display_name, _(" (invalid encoding)"), NULL);
-          g_free (p);
-        }
-      g_file_info_set_display_name (info, display_name);
-      g_free (display_name);
-    }
-
   if (g_file_attribute_matcher_matches (matcher,
+                                        G_FILE_ATTRIBUTE_STANDARD_DISPLAY_NAME) ||
+      g_file_attribute_matcher_matches (matcher,
                                         G_FILE_ATTRIBUTE_STANDARD_EDIT_NAME))
     {
-      char *edit_name = g_filename_display_name (basename);
-      g_file_info_set_edit_name (info, edit_name);
+      char *edit_name = gvfs_file_info_populate_names_as_local (info, basename);
       g_free (edit_name);
     }
 
@@ -783,11 +778,25 @@ typedef struct
   struct nfsfh *srcfh;
   struct nfsfh *destfh;
   char *dest;
+  int mode;
   CopyFileCallback cb;
   void *private_data;
 } CopyHandle;
 
 static void
+copy_handle_complete (struct nfs_context *ctx,
+                      CopyHandle *handle,
+                      gboolean result)
+{
+  if (handle->srcfh)
+    nfs_close_async (ctx, handle->srcfh, null_cb, NULL);
+  if (handle->destfh)
+    nfs_close_async (ctx, handle->destfh, null_cb, NULL);
+  handle->cb (result, handle->private_data);
+  g_slice_free (CopyHandle, handle);
+}
+
+static void
 copy_read_cb (int err, struct nfs_context *ctx, void *data, void *private_data);
 
 static void
@@ -799,16 +808,9 @@ copy_write_cb (int err,
   CopyHandle *handle = private_data;
 
   if (err > 0)
-    {
-      nfs_read_async (ctx, handle->srcfh, COPY_BLKSIZE, copy_read_cb, handle);
-    }
+    nfs_read_async (ctx, handle->srcfh, COPY_BLKSIZE, copy_read_cb, handle);
   else
-    {
-      nfs_close_async (ctx, handle->srcfh, null_cb, NULL);
-      nfs_close_async (ctx, handle->destfh, null_cb, NULL);
-      handle->cb (FALSE, handle->private_data);
-      g_slice_free (CopyHandle, handle);
-    }
+    copy_handle_complete (ctx, handle, FALSE);
 }
 
 static void
@@ -817,23 +819,11 @@ copy_read_cb (int err, struct nfs_context *ctx, void *data, void *private_data)
   CopyHandle *handle = private_data;
 
   if (err == 0)
-    {
-      nfs_close_async (ctx, handle->srcfh, null_cb, NULL);
-      nfs_close_async (ctx, handle->destfh, null_cb, NULL);
-      handle->cb (TRUE, handle->private_data);
-      g_slice_free (CopyHandle, handle);
-    }
+    copy_handle_complete (ctx, handle, TRUE);
   else if (err > 0)
-    {
-      nfs_write_async (ctx, handle->destfh, err, data, copy_write_cb, handle);
-    }
+    nfs_write_async (ctx, handle->destfh, err, data, copy_write_cb, handle);
   else
-    {
-      nfs_close_async (ctx, handle->srcfh, null_cb, NULL);
-      nfs_close_async (ctx, handle->destfh, null_cb, NULL);
-      handle->cb (FALSE, handle->private_data);
-      g_slice_free (CopyHandle, handle);
-    }
+    copy_handle_complete (ctx, handle, FALSE);
 }
 
 static void
@@ -851,9 +841,7 @@ copy_open_dest_cb (int err,
     }
   else
     {
-      nfs_close_async (ctx, handle->srcfh, null_cb, NULL);
-      handle->cb (FALSE, handle->private_data);
-      g_slice_free (CopyHandle, handle);
+      copy_handle_complete (ctx, handle, FALSE);
     }
 }
 
@@ -868,27 +856,27 @@ copy_open_source_cb (int err,
     {
       handle->srcfh = data;
       nfs_create_async (ctx,
-                        handle->dest, O_TRUNC, 0600,
+                        handle->dest, O_TRUNC, handle->mode & 0777,
                         copy_open_dest_cb, handle);
       g_free (handle->dest);
     }
   else
     {
-      handle->cb (FALSE, handle->private_data);
       g_free (handle->dest);
-      g_slice_free (CopyHandle, handle);
+      copy_handle_complete (ctx, handle, FALSE);
     }
 }
 
 static void
 copy_file (struct nfs_context *ctx,
-           const char *src, const char *dest,
+           const char *src, const char *dest, int mode,
            CopyFileCallback cb, void *private_data)
 {
   CopyHandle *handle;
 
   handle = g_slice_new0 (CopyHandle);
   handle->dest = g_strdup (dest);
+  handle->mode = mode;
   handle->cb = cb;
   handle->private_data = private_data;
 
@@ -934,7 +922,7 @@ replace_backup_chown_cb (int err,
   g_free (handle->backup_filename);
   handle->backup_filename = NULL;
 
-  if (err == 0)
+  if (err == 0 || err == -EPERM)
     {
       GVfsJobOpenForWrite *op_job = G_VFS_JOB_OPEN_FOR_WRITE (job);
       GVfsBackendNfs *op_backend = G_VFS_BACKEND_NFS (op_job->backend);
@@ -955,29 +943,6 @@ replace_backup_chown_cb (int err,
 }
 
 static void
-replace_backup_chmod_cb (int err,
-                         struct nfs_context *ctx,
-                         void *data, void *private_data)
-{
-  WriteHandle *handle = private_data;
-  GVfsJob *job = handle->job;
-
-  if (err == 0)
-    {
-      nfs_chown_async (ctx,
-                       handle->backup_filename, handle->uid, handle->gid,
-                       replace_backup_chown_cb, handle);
-    }
-  else
-    {
-      g_vfs_job_failed_literal (job,
-                                G_IO_ERROR, G_IO_ERROR_CANT_CREATE_BACKUP,
-                                _("Backup file creation failed"));
-      write_handle_free (handle);
-    }
-}
-
-static void
 replace_backup_cb (gboolean success, void *private_data)
 {
   WriteHandle *handle = private_data;
@@ -987,10 +952,9 @@ replace_backup_cb (gboolean success, void *private_data)
     {
       GVfsJobOpenForWrite *op_job = G_VFS_JOB_OPEN_FOR_WRITE (job);
       GVfsBackendNfs *op_backend = G_VFS_BACKEND_NFS (op_job->backend);
-
-      nfs_chmod_async (op_backend->ctx,
-                       handle->backup_filename, handle->mode & 0777,
-                       replace_backup_chmod_cb, handle);
+      nfs_chown_async (op_backend->ctx,
+                       handle->backup_filename, handle->uid, handle->gid,
+                       replace_backup_chown_cb, handle);
     }
   else
     {
@@ -1009,12 +973,13 @@ replace_rm_backup_cb (int err,
   WriteHandle *handle = private_data;
   GVfsJob *job = handle->job;
 
-  if (err == 0 || err == -ENOENT)
+  if (err == 0 || err == -ENOENT || err == -EACCES)
     {
       GVfsJobOpenForWrite *op_job = G_VFS_JOB_OPEN_FOR_WRITE (job);
 
       copy_file (ctx,
                  op_job->filename, handle->backup_filename,
+                 handle->mode & 0777,
                  replace_backup_cb, handle);
     }
   else
@@ -1161,90 +1126,43 @@ replace_temp_cb (int err,
 }
 
 static void
-replace_lstat_cb (int err,
-                  struct nfs_context *ctx,
-                  void *data, void *private_data)
-{
-  WriteHandle *handle = private_data;
-  GVfsJob *job = handle->job;
-
-  if (err == 0)
-    {
-      GVfsJobOpenForWrite *op_job = G_VFS_JOB_OPEN_FOR_WRITE (job);
-      GVfsBackendNfs *op_backend = G_VFS_BACKEND_NFS (op_job->backend);
-      struct nfs_stat_64 *st = data;
-
-      handle->is_symlink = S_ISLNK (st->nfs_mode);
-
-      if ((op_job->flags & G_FILE_CREATE_REPLACE_DESTINATION) &&
-          !handle->is_symlink && S_ISDIR (handle->mode))
-        {
-          g_vfs_job_failed_literal (job,
-                                    G_IO_ERROR, G_IO_ERROR_IS_DIRECTORY,
-                                    _("Target file is a directory"));
-          write_handle_free (handle);
-        }
-      else if ((op_job->flags & G_FILE_CREATE_REPLACE_DESTINATION) ||
-               (!handle->is_symlink && handle->nlink <= 1))
-        {
-          char *dirname;
-          char basename[] = ".giosaveXXXXXX";
-
-          handle->filename = g_strdup (op_job->filename);
-
-          dirname = g_path_get_dirname (op_job->filename);
-          gvfs_randomize_string (basename + 8, 6);
-          handle->tempname = g_build_filename (dirname, basename, NULL);
-          g_free (dirname);
-
-          nfs_create_async (ctx,
-                            handle->tempname,
-                            O_EXCL,
-                            (op_job->flags & G_FILE_CREATE_PRIVATE ? 0600 : 0666) & ~op_backend->umask,
-                            replace_temp_cb, handle);
-        }
-      else
-        {
-          replace_truncate (ctx, handle);
-        }
-    }
-  else
-    {
-      g_vfs_job_failed_from_errno (job, -err);
-      write_handle_free (handle);
-    }
-}
-
-static void
 replace_stat_cb (int err,
                  struct nfs_context *ctx,
                  void *data, void *private_data)
 {
-  GVfsJob *job = G_VFS_JOB (private_data);
+  WriteHandle *handle = private_data;
+  GVfsJob *job = handle->job;
 
   if (err == 0)
     {
       GVfsJobOpenForWrite *op_job = G_VFS_JOB_OPEN_FOR_WRITE (job);
+      GVfsBackendNfs *op_backend = G_VFS_BACKEND_NFS (op_job->backend);
       struct nfs_stat_64 *st = data;
 
-      if (!(op_job->flags & G_FILE_CREATE_REPLACE_DESTINATION) &&
+      /* Fail if we're not replacing the destination and the destination is a
+       * directory, or if we are replacing the destination and the destination
+       * is a directory */
+      if ((!(op_job->flags & G_FILE_CREATE_REPLACE_DESTINATION) ||
+           !handle->is_symlink) &&
           S_ISDIR (st->nfs_mode))
         {
           g_vfs_job_failed_literal (job,
                                     G_IO_ERROR, G_IO_ERROR_IS_DIRECTORY,
                                     _("Target file is a directory"));
+          write_handle_free (handle);
         }
+      /* Fail if we're not replacing the destination and the destination is not
+       * a regular file. */
       else if (!(op_job->flags & G_FILE_CREATE_REPLACE_DESTINATION) &&
                !S_ISREG (st->nfs_mode))
         {
           g_vfs_job_failed_literal (job,
                                     G_IO_ERROR, G_IO_ERROR_NOT_REGULAR_FILE,
                                     _("Target file is not a regular file"));
+          write_handle_free (handle);
         }
       else
         {
-          WriteHandle *handle;
-
           if (op_job->etag)
             {
               char *etag;
@@ -1256,24 +1174,82 @@ replace_stat_cb (int err,
                   g_vfs_job_failed_literal (job,
                                             G_IO_ERROR, G_IO_ERROR_WRONG_ETAG,
                                             _("The file was externally modified"));
+                  write_handle_free (handle);
                   return;
                 }
               g_free (etag);
             }
 
-          handle = g_slice_new0 (WriteHandle);
           handle->mode = st->nfs_mode;
           handle->uid = st->nfs_uid;
           handle->gid = st->nfs_gid;
           handle->nlink = st->nfs_nlink;
-          handle->job = g_object_ref (job);
 
-          nfs_lstat64_async (ctx, op_job->filename, replace_lstat_cb, handle);
+          /* Write to a temporary file then do an atomic rename if either:
+           * - G_FILE_CREATE_REPLACE_DESTINATION is specified
+           * - the destination is not a symlink, and it does not have multiple
+           *   hard links.
+           */
+          if ((op_job->flags & G_FILE_CREATE_REPLACE_DESTINATION) ||
+              (!handle->is_symlink && handle->nlink <= 1))
+            {
+              char *dirname;
+              char basename[] = ".giosaveXXXXXX";
+
+              handle->filename = g_strdup (op_job->filename);
+
+              dirname = g_path_get_dirname (op_job->filename);
+              gvfs_randomize_string (basename + 8, 6);
+              handle->tempname = g_build_filename (dirname, basename, NULL);
+              g_free (dirname);
+
+              nfs_create_async (ctx,
+                                handle->tempname,
+                                O_EXCL,
+                                (op_job->flags & G_FILE_CREATE_PRIVATE ? 0600 : 0666) & ~op_backend->umask,
+                                replace_temp_cb, handle);
+            }
+          else
+            {
+              replace_truncate (ctx, handle);
+            }
         }
     }
   else
     {
       g_vfs_job_failed_from_errno (job, -err);
+      write_handle_free (handle);
+    }
+}
+
+static void
+replace_lstat_cb (int err,
+                  struct nfs_context *ctx,
+                  void *data, void *private_data)
+{
+  GVfsJob *job = G_VFS_JOB (private_data);
+
+  if (err == 0)
+    {
+      WriteHandle *handle;
+      GVfsJobOpenForWrite *op_job = G_VFS_JOB_OPEN_FOR_WRITE (job);
+      struct nfs_stat_64 *st = data;
+
+      handle = g_slice_new0 (WriteHandle);
+      handle->is_symlink = S_ISLNK (st->nfs_mode);
+      handle->job = g_object_ref (job);
+
+      /* If the filename is a link, call stat to get the real info.
+       * Otherwise, lstat is the same as stat, so just chain straight to the
+       * stat callback. */
+      if (handle->is_symlink)
+        nfs_stat64_async (ctx, op_job->filename, replace_stat_cb, handle);
+      else
+        replace_stat_cb (err, ctx, data, handle);
+    }
+  else
+    {
+      g_vfs_job_failed_from_errno (job, -err);
     }
 }
 
@@ -1297,7 +1273,7 @@ replace_create_cb (int err,
     }
   else if (err == -EEXIST)
     {
-      nfs_stat64_async (ctx, op_job->filename, replace_stat_cb, job);
+      nfs_lstat64_async (ctx, op_job->filename, replace_lstat_cb, job);
     }
   else
     {
@@ -1757,13 +1733,6 @@ enumerate_readlink_cb (int err,
   enumerate_continue (handle, ctx);
 }
 
-static char *
-build_filename (GVfsJobEnumerate *op_job, GFileInfo *item)
-{
-  const char *filename = g_file_info_get_name (item);
-  return g_build_filename (op_job->filename, filename, NULL);
-}
-
 static void
 enumerate_continue (EnumerateHandle *handle, struct nfs_context *ctx)
 {
@@ -1773,17 +1742,20 @@ enumerate_continue (EnumerateHandle *handle, struct nfs_context *ctx)
 
       if (handle->readlink_list)
         {
-          path = build_filename (handle->op_job, handle->readlink_list->data);
+          const char *filename = g_file_info_get_name (handle->readlink_list->data);
+          path = g_build_filename (handle->op_job->filename, filename, NULL);
           nfs_readlink_async (ctx, path, enumerate_readlink_cb, handle);
         }
       else if (handle->symlink_list)
         {
-          path = build_filename (handle->op_job, handle->symlink_list->data);
+          const char *filename = g_file_info_get_name (handle->symlink_list->data);
+          path = g_build_filename (handle->op_job->filename, filename, NULL);
           nfs_stat64_async (ctx, path, enumerate_stat_cb, handle);
         }
       else if (handle->access_list)
         {
-          path = build_filename (handle->op_job, handle->access_list->data);
+          const char *filename = g_file_info_get_name (handle->access_list->data);
+          path = g_build_filename (handle->op_job->filename, filename, NULL);
           nfs_access2_async (ctx, path, enumerate_access_cb, handle);
         }
 
@@ -2062,7 +2034,14 @@ stat_is_symlink_cb (int err,
       struct nfs_stat_64 *st = data;
 
       g_file_info_set_is_symlink (op_job->file_info, S_ISLNK (st->nfs_mode));
-      nfs_stat64_async (ctx, op_job->filename, stat_cb, job);
+
+      /* If the filename is a link, call stat to get the real info.
+       * Otherwise, lstat is the same as stat, so just chain straight to the
+       * stat callback. */
+      if (S_ISLNK (st->nfs_mode))
+        nfs_stat64_async (ctx, op_job->filename, stat_cb, job);
+      else
+        stat_cb (err, ctx, data, private_data);
     }
   else
     {
@@ -2404,9 +2383,9 @@ move_stat_dest_cb (int err,
         }
       else
         {
-         g_vfs_job_failed (job,
-                           G_IO_ERROR, G_IO_ERROR_EXISTS,
-                           _("Target file already exists"));
+          g_vfs_job_failed (job,
+                            G_IO_ERROR, G_IO_ERROR_EXISTS,
+                            _("Target file already exists"));
           g_slice_free (MoveHandle, handle);
           return;
         }
diff --git a/daemon/gvfsbackendnfs.h b/daemon/gvfsbackendnfs.h
index 169ad75..3d239be 100644
--- a/daemon/gvfsbackendnfs.h
+++ b/daemon/gvfsbackendnfs.h
@@ -1,5 +1,5 @@
 /* GIO - GLib Input, Output and Streaming Library
- * 
+ *
  * Copyright (C) 2014 Ross Lagerwall
  *
  * This library is free software; you can redistribute it and/or
@@ -13,9 +13,8 @@
  * Lesser General Public License for more details.
  *
  * You should have received a copy of the GNU Lesser General
- * Public License along with this library; if not, write to the
- * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
- * Boston, MA 02110-1301, USA.
+ * Public License along with this library; if not, see
+ * <http://www.gnu.org/licenses/>.
  */
 
 #ifndef __G_VFS_BACKEND_NFS_H__