{"action":"opened","issue":{"url":"https://api.github.com/repos/Netatalk/Netatalk/issues/37","repository_url":"https://api.github.com/repos/Netatalk/Netatalk","labels_url":"https://api.github.com/repos/Netatalk/Netatalk/issues/37/labels{/name}","comments_url":"https://api.github.com/repos/Netatalk/Netatalk/issues/37/comments","events_url":"https://api.github.com/repos/Netatalk/Netatalk/issues/37/events","html_url":"https://github.com/Netatalk/Netatalk/issues/37","id":134390361,"number":37,"title":"[PATCH] Cleanup copyfile code","user":{"login":"andy-js","id":1582704,"avatar_url":"https://avatars.githubusercontent.com/u/1582704?v=3","gravatar_id":"","url":"https://api.github.com/users/andy-js","html_url":"https://github.com/andy-js","followers_url":"https://api.github.com/users/andy-js/followers","following_url":"https://api.github.com/users/andy-js/following{/other_user}","gists_url":"https://api.github.com/users/andy-js/gists{/gist_id}","starred_url":"https://api.github.com/users/andy-js/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/andy-js/subscriptions","organizations_url":"https://api.github.com/users/andy-js/orgs","repos_url":"https://api.github.com/users/andy-js/repos","events_url":"https://api.github.com/users/andy-js/events{/privacy}","received_events_url":"https://api.github.com/users/andy-js/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":0,"created_at":"2016-02-17T20:24:52Z","updated_at":"2016-02-17T20:24:52Z","closed_at":null,"body":"These changes cleanup the copyfile code which uses two variables where one will do (err and err_ret) and does not use them consistently (sometimes storing generic error values and other times storing errno values).  I've also added comments that describe the invocation of the vfs copyfile functions.\r\n\r\n<pre>\r\ndiff --git a/etc/afpd/file.c b/etc/afpd/file.c\r\nindex 27e5212..1d4bc30 100644\r\n--- a/etc/afpd/file.c\r\n+++ b/etc/afpd/file.c\r\n@@ -1403,7 +1403,6 @@ int copyfile(struct vol *s_vol,\r\n {\r\n     struct adouble\tads, add;\r\n     int\t\t\terr = 0;\r\n-    int                 ret_err = 0;\r\n     int                 adflags;\r\n     int                 stat_result;\r\n     struct stat         st;\r\n@@ -1419,7 +1418,7 @@ int copyfile(struct vol *s_vol,\r\n     adflags = ADFLAGS_DF | ADFLAGS_HF | ADFLAGS_NOHF | ADFLAGS_RF | ADFLAGS_NORF;\r\n \r\n     if (ad_openat(adp, sfd, src, adflags | ADFLAGS_RDONLY) < 0) {\r\n-        ret_err = errno;\r\n+        err = errno;\r\n         goto done;\r\n     }\r\n \r\n@@ -1440,26 +1439,24 @@ int copyfile(struct vol *s_vol,\r\n \r\n     ad_init(&add, d_vol);\r\n     if (ad_open(&add, dst, adflags | ADFLAGS_RDWR | ADFLAGS_CREATE | ADFLAGS_EXCL, st.st_mode | S_IRUSR | S_IWUSR) < 0) {\r\n-        ret_err = errno;\r\n+        err = errno;\r\n         ad_close( adp, adflags );\r\n-        if (EEXIST != ret_err) {\r\n+        if (EEXIST != err) {\r\n             deletefile(d_vol, -1, dst, 0);\r\n             goto done;\r\n         }\r\n         return AFPERR_EXIST;\r\n     }\r\n \r\n-    if ((err = copy_fork(ADEID_DFORK, &add, adp,\r\n+    if (copy_fork(ADEID_DFORK, &add, adp,\r\n                          s_vol->v_obj->dsi->commands,\r\n-                         s_vol->v_obj->dsi->server_quantum)) != 0)\r\n-        LOG(log_error, logtype_afpd, \"copyfile('%s'): %s\", src, strerror(errno));\r\n-\r\n-    if (err == 0)\r\n-        if ((err = d_vol->vfs->vfs_copyfile(d_vol, sfd, src, dst)) != 0)\r\n-            LOG(log_error, logtype_afpd, \"copyfile('%s'): %s\", src, strerror(errno));\r\n-\r\n-    if (err < 0)\r\n-       ret_err = errno;\r\n+                         s_vol->v_obj->dsi->server_quantum) != 0) {\r\n+        err = errno;\r\n+        LOG(log_error, logtype_afpd, \"copyfile('%s'): copy_fork: %s\", src, strerror(errno));\r\n+    } else if (d_vol->vfs->vfs_copyfile(d_vol, sfd, src, dst) != AFP_OK) {\r\n+        err = errno;\r\n+        LOG(log_error, logtype_afpd, \"copyfile('%s'): vfs_copyfile: %s\", src, strerror(errno));\r\n+    }\r\n \r\n     if (AD_META_OPEN(&add)) {\r\n         if (AD_META_OPEN(adp))\r\n@@ -1468,11 +1465,11 @@ int copyfile(struct vol *s_vol,\r\n         cnid_t id;\r\n         struct stat stdest;\r\n         if (fstat(ad_meta_fileno(&add), &stdest) != 0) {\r\n-            ret_err = errno;\r\n+            err = errno;\r\n             goto error;\r\n         }\r\n         if ((id = get_id(d_vol, &add, &stdest, d_dir->d_did, dst, strlen(dst))) == CNID_INVALID) {\r\n-            ret_err = EINVAL;\r\n+            err = EINVAL;\r\n             goto error;\r\n         }\r\n         (void)ad_setid(&add, stdest.st_dev, stdest.st_ino, id, d_dir->d_did, d_vol->v_stamp);\r\n@@ -1483,10 +1480,10 @@ error:\r\n     ad_close( adp, adflags );\r\n \r\n     if (ad_close( &add, adflags ) <0) {\r\n-       ret_err = errno;\r\n+       err = errno;\r\n     } \r\n \r\n-    if (ret_err) {\r\n+    if (err) {\r\n         deletefile(d_vol, -1, dst, 0);\r\n     }\r\n     else if (stat_result == 0) {\r\n@@ -1501,7 +1498,7 @@ error:\r\n     }\r\n \r\n done:\r\n-    switch ( ret_err ) {\r\n+    switch ( err ) {\r\n     case 0:\r\n         return AFP_OK;\r\n     case EDQUOT:\r\ndiff --git a/libatalk/vfs/ea_ad.c b/libatalk/vfs/ea_ad.c\r\nindex 5b99a5c..c7b6381 100644\r\n--- a/libatalk/vfs/ea_ad.c\r\n+++ b/libatalk/vfs/ea_ad.c\r\n@@ -1476,6 +1476,24 @@ exit:\r\n \treturn ret;\r\n }\r\n \r\n+/*\r\n+ * Function: ea_copyfile\r\n+ *\r\n+ * Purpose: copy EAs\r\n+ *\r\n+ * Arguments:\r\n+ *\r\n+ *    vol          (r) current volume\r\n+ *    sdf          (r) source file descriptor\r\n+ *    src          (r) source path\r\n+ *    dst          (r) destination path\r\n+ *\r\n+ * Return AFP code AFP_OK on success or appropriate AFP error code\r\n+ *\r\n+ * Effects:\r\n+ *\r\n+ * Copies EAs from source file to dest file.\r\n+ */\r\n int ea_copyfile(VFS_FUNC_ARGS_COPYFILE)\r\n {\r\n     unsigned int count = 0;\r\ndiff --git a/libatalk/vfs/ea_sys.c b/libatalk/vfs/ea_sys.c\r\nindex 7415302..3f6b599 100644\r\n--- a/libatalk/vfs/ea_sys.c\r\n+++ b/libatalk/vfs/ea_sys.c\r\n@@ -465,9 +465,22 @@ int sys_remove_ea(VFS_FUNC_ARGS_EA_REMOVE)\r\n }\r\n \r\n /*\r\n- * @brief Copy EAs\r\n+ * Function: sys_ea_copyfile\r\n  *\r\n- * @note Supports *at semantics, therfor switches back and forth between sfd and cwd\r\n+ * Purpose: copy EAs\r\n+ *\r\n+ * Arguments:\r\n+ *\r\n+ *    vol          (r) current volume\r\n+ *    sdf          (r) source file descriptor\r\n+ *    src          (r) source path\r\n+ *    dst          (r) destination path\r\n+ *\r\n+ * Return AFP code AFP_OK on success or appropriate AFP error code\r\n+ *\r\n+ * Effects:\r\n+ *\r\n+ * Copies EAs from source file to dest file.\r\n  */\r\n int sys_ea_copyfile(VFS_FUNC_ARGS_COPYFILE)\r\n {\r\n</pre>"}}