]> git.ipfire.org Git - thirdparty/samba.git/commitdiff
smbd: use an early exit if stat succeeds in unix_convert_step_stat()
authorRalph Boehme <slow@samba.org>
Thu, 23 Apr 2020 09:46:19 +0000 (11:46 +0200)
committerJeremy Allison <jra@samba.org>
Fri, 24 Apr 2020 21:46:27 +0000 (21:46 +0000)
Allows decreasing the indentation level of the bulk of the code that handles
stat failure. Best viewed with `git show -w`.

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
source3/smbd/filename.c

index ed6a5c95c41da263738ae246275028f021462f07..75916c88d2a1deeea7a836602d54b69a5b0d047b 100644 (file)
@@ -478,6 +478,7 @@ struct uc_state {
 
 static NTSTATUS unix_convert_step_stat(struct uc_state *state)
 {
+       char *found_name = NULL;
        int ret;
 
        /*
@@ -511,241 +512,182 @@ static NTSTATUS unix_convert_step_stat(struct uc_state *state)
                         */
                        return NT_STATUS_OBJECT_PATH_NOT_FOUND;
                }
+               return NT_STATUS_OK;
+       }
 
-       } else {
-               char *found_name = NULL;
+       /* Stat failed - ensure we don't use it. */
+       SET_STAT_INVALID(state->smb_fname->st);
 
-               /* Stat failed - ensure we don't use it. */
-               SET_STAT_INVALID(state->smb_fname->st);
-
-               if (state->posix_pathnames) {
+       if (state->posix_pathnames) {
+               /*
+                * For posix_pathnames, we're done.
+                * Don't blunder into the name_has_wildcard OR
+                * get_real_filename() codepaths as they may
+                * be doing case insensitive lookups. So when
+                * creating a new POSIX directory Foo they might
+                * match on name foo.
+                *
+                * BUG: https://bugzilla.samba.org/show_bug.cgi?id=13803
+                */
+               if (state->end != NULL) {
+                       const char *morepath = NULL;
                        /*
-                        * For posix_pathnames, we're done.
-                        * Don't blunder into the name_has_wildcard OR
-                        * get_real_filename() codepaths as they may
-                        * be doing case insensitive lookups. So when
-                        * creating a new POSIX directory Foo they might
-                        * match on name foo.
-                        *
-                        * BUG: https://bugzilla.samba.org/show_bug.cgi?id=13803
+                        * If this is intermediate we must
+                        * restore the full path.
                         */
-                       if (state->end != NULL) {
-                               const char *morepath = NULL;
-                               /*
-                                * If this is intermediate we must
-                                * restore the full path.
-                                */
-                               *state->end = '/';
-                               /*
-                                * If there are any more components
-                                * after the failed LSTAT we cannot
-                                * continue.
-                                */
-                               morepath = strchr(state->end + 1, '/');
-                               if (morepath != NULL) {
-                                       return NT_STATUS_OBJECT_PATH_NOT_FOUND;
-                               }
-                       }
-                       if (errno == ENOENT) {
-                               /* New file or directory. */
-                               state->done = true;
-                               return NT_STATUS_OK;
-                       }
-                       if ((errno == EACCES) &&
-                           (state->ucf_flags & UCF_PREP_CREATEFILE)) {
-                               /* POSIX Dropbox case. */
-                               errno = 0;
-                               state->done = true;
-                               return NT_STATUS_OK;
+                       *state->end = '/';
+                       /*
+                        * If there are any more components
+                        * after the failed LSTAT we cannot
+                        * continue.
+                        */
+                       morepath = strchr(state->end + 1, '/');
+                       if (morepath != NULL) {
+                               return NT_STATUS_OBJECT_PATH_NOT_FOUND;
                        }
-                       return map_nt_error_from_unix(errno);
                }
+               if (errno == ENOENT) {
+                       /* New file or directory. */
+                       state->done = true;
+                       return NT_STATUS_OK;
+               }
+               if ((errno == EACCES) &&
+                   (state->ucf_flags & UCF_PREP_CREATEFILE)) {
+                       /* POSIX Dropbox case. */
+                       errno = 0;
+                       state->done = true;
+                       return NT_STATUS_OK;
+               }
+               return map_nt_error_from_unix(errno);
+       }
 
-               /*
-                * Reset errno so we can detect
-                * directory open errors.
-                */
-               errno = 0;
-
-               /*
-                * Try to find this part of the path in the directory.
-                */
-
-               if (state->name_has_wildcard ||
-                   (get_real_filename(state->conn, state->dirpath, state->name,
-                                      talloc_tos(),
-                                      &found_name) == -1)) {
-                       char *unmangled;
+       /*
+        * Reset errno so we can detect
+        * directory open errors.
+        */
+       errno = 0;
 
-                       if (state->end) {
-                               /*
-                                * An intermediate part of the name
-                                * can't be found.
-                                */
-                               DBG_DEBUG("Intermediate [%s] missing\n",
-                                         state->name);
-                               *state->end = '/';
+       /*
+        * Try to find this part of the path in the directory.
+        */
 
-                               /*
-                                * We need to return the fact that the
-                                * intermediate name resolution failed.
-                                * This is used to return an error of
-                                * ERRbadpath rather than ERRbadfile.
-                                * Some Windows applications depend on
-                                * the difference between these two
-                                * errors.
-                                */
+       if (state->name_has_wildcard ||
+           (get_real_filename(state->conn, state->dirpath, state->name,
+                              talloc_tos(),
+                              &found_name) == -1)) {
+               char *unmangled;
 
-                               /*
-                                * ENOENT, ENOTDIR and ELOOP all map
-                                * to NT_STATUS_OBJECT_PATH_NOT_FOUND
-                                * in the filename walk.
-                                */
+               if (state->end) {
+                       /*
+                        * An intermediate part of the name
+                        * can't be found.
+                        */
+                       DBG_DEBUG("Intermediate [%s] missing\n",
+                                 state->name);
+                       *state->end = '/';
 
-                               if (errno == ENOENT ||
-                                   errno == ENOTDIR ||
-                                   errno == ELOOP)
-                               {
-                                       return NT_STATUS_OBJECT_PATH_NOT_FOUND;
-                               }
-                               return map_nt_error_from_unix(errno);
-                       }
+                       /*
+                        * We need to return the fact that the
+                        * intermediate name resolution failed.
+                        * This is used to return an error of
+                        * ERRbadpath rather than ERRbadfile.
+                        * Some Windows applications depend on
+                        * the difference between these two
+                        * errors.
+                        */
 
                        /*
-                        * ENOENT/EACCESS are the only valid errors
-                        * here.
+                        * ENOENT, ENOTDIR and ELOOP all map
+                        * to NT_STATUS_OBJECT_PATH_NOT_FOUND
+                        * in the filename walk.
                         */
 
-                       if (errno == EACCES) {
-                               if ((state->ucf_flags & UCF_PREP_CREATEFILE) == 0) {
-                                       return NT_STATUS_ACCESS_DENIED;
-                               } else {
-                                       /*
-                                        * This is the dropbox
-                                        * behaviour. A dropbox is a
-                                        * directory with only -wx
-                                        * permissions, so
-                                        * get_real_filename fails
-                                        * with EACCESS, it needs to
-                                        * list the directory. We
-                                        * nevertheless want to allow
-                                        * users creating a file.
-                                        */
-                                       errno = 0;
-                               }
+                       if (errno == ENOENT ||
+                           errno == ENOTDIR ||
+                           errno == ELOOP)
+                       {
+                               return NT_STATUS_OBJECT_PATH_NOT_FOUND;
                        }
+                       return map_nt_error_from_unix(errno);
+               }
+
+               /*
+                * ENOENT/EACCESS are the only valid errors
+                * here.
+                */
 
-                       if ((errno != 0) && (errno != ENOENT)) {
+               if (errno == EACCES) {
+                       if ((state->ucf_flags & UCF_PREP_CREATEFILE) == 0) {
+                               return NT_STATUS_ACCESS_DENIED;
+                       } else {
                                /*
-                                * ENOTDIR and ELOOP both map to
-                                * NT_STATUS_OBJECT_PATH_NOT_FOUND
-                                * in the filename walk.
+                                * This is the dropbox
+                                * behaviour. A dropbox is a
+                                * directory with only -wx
+                                * permissions, so
+                                * get_real_filename fails
+                                * with EACCESS, it needs to
+                                * list the directory. We
+                                * nevertheless want to allow
+                                * users creating a file.
                                 */
-                               if (errno == ENOTDIR || errno == ELOOP) {
-                                       return NT_STATUS_OBJECT_PATH_NOT_FOUND;
-                               }
-                               return map_nt_error_from_unix(errno);
+                               errno = 0;
                        }
+               }
 
+               if ((errno != 0) && (errno != ENOENT)) {
                        /*
-                        * Just the last part of the name doesn't exist.
-                        * We need to strupper() or strlower() it as
-                        * this conversion may be used for file creation
-                        * purposes. Fix inspired by
-                        * Thomas Neumann <t.neumann@iku-ag.de>.
+                        * ENOTDIR and ELOOP both map to
+                        * NT_STATUS_OBJECT_PATH_NOT_FOUND
+                        * in the filename walk.
                         */
-                       if (!state->conn->case_preserve ||
-                           (mangle_is_8_3(state->name, false,
-                                          state->conn->params) &&
-                            !state->conn->short_case_preserve)) {
-                               if (!strnorm(state->name,
-                                            lp_default_case(SNUM(state->conn)))) {
-                                       DBG_DEBUG("strnorm %s failed\n",
-                                                 state->name);
-                                       return NT_STATUS_INVALID_PARAMETER;
-                               }
+                       if (errno == ENOTDIR || errno == ELOOP) {
+                               return NT_STATUS_OBJECT_PATH_NOT_FOUND;
                        }
+                       return map_nt_error_from_unix(errno);
+               }
 
-                       /*
-                        * check on the mangled stack to see if we can
-                        * recover the base of the filename.
-                        */
-
-                       if (mangle_is_mangled(state->name, state->conn->params)
-                           && mangle_lookup_name_from_8_3(state->mem_ctx,
-                                                          state->name,
-                                                          &unmangled,
-                                                          state->conn->params)) {
-                               char *tmp;
-                               size_t name_ofs =
-                                       state->name - state->smb_fname->base_name;
-
-                               if (!ISDOT(state->dirpath)) {
-                                       tmp = talloc_asprintf(
-                                               state->smb_fname, "%s/%s",
-                                               state->dirpath, unmangled);
-                                       TALLOC_FREE(unmangled);
-                               }
-                               else {
-                                       tmp = unmangled;
-                               }
-                               if (tmp == NULL) {
-                                       DBG_ERR("talloc failed\n");
-                                       return NT_STATUS_NO_MEMORY;
-                               }
-                               TALLOC_FREE(state->smb_fname->base_name);
-                               state->smb_fname->base_name = tmp;
-                               state->name =
-                                       state->smb_fname->base_name + name_ofs;
-                               state->end = state->name + strlen(state->name);
+               /*
+                * Just the last part of the name doesn't exist.
+                * We need to strupper() or strlower() it as
+                * this conversion may be used for file creation
+                * purposes. Fix inspired by
+                * Thomas Neumann <t.neumann@iku-ag.de>.
+                */
+               if (!state->conn->case_preserve ||
+                   (mangle_is_8_3(state->name, false,
+                                  state->conn->params) &&
+                    !state->conn->short_case_preserve)) {
+                       if (!strnorm(state->name,
+                                    lp_default_case(SNUM(state->conn)))) {
+                               DBG_DEBUG("strnorm %s failed\n",
+                                         state->name);
+                               return NT_STATUS_INVALID_PARAMETER;
                        }
-
-                       DBG_DEBUG("New file [%s]\n", state->name);
-                       state->done = true;
-                       return NT_STATUS_OK;
                }
 
-
                /*
-                * Restore the rest of the string. If the string was
-                * mangled the size may have changed.
+                * check on the mangled stack to see if we can
+                * recover the base of the filename.
                 */
-               if (state->end) {
+
+               if (mangle_is_mangled(state->name, state->conn->params)
+                   && mangle_lookup_name_from_8_3(state->mem_ctx,
+                                                  state->name,
+                                                  &unmangled,
+                                                  state->conn->params)) {
                        char *tmp;
                        size_t name_ofs =
                                state->name - state->smb_fname->base_name;
 
                        if (!ISDOT(state->dirpath)) {
-                               tmp = talloc_asprintf(state->smb_fname,
-                                                     "%s/%s/%s", state->dirpath,
-                                                     found_name, state->end+1);
+                               tmp = talloc_asprintf(
+                                       state->smb_fname, "%s/%s",
+                                       state->dirpath, unmangled);
+                               TALLOC_FREE(unmangled);
                        }
                        else {
-                               tmp = talloc_asprintf(state->smb_fname,
-                                                     "%s/%s", found_name,
-                                                     state->end+1);
-                       }
-                       if (tmp == NULL) {
-                               DBG_ERR("talloc_asprintf failed\n");
-                               return NT_STATUS_NO_MEMORY;
-                       }
-                       TALLOC_FREE(state->smb_fname->base_name);
-                       state->smb_fname->base_name = tmp;
-                       state->name = state->smb_fname->base_name + name_ofs;
-                       state->end = state->name + strlen(found_name);
-                       *state->end = '\0';
-               } else {
-                       char *tmp;
-                       size_t name_ofs =
-                               state->name - state->smb_fname->base_name;
-
-                       if (!ISDOT(state->dirpath)) {
-                               tmp = talloc_asprintf(state->smb_fname,
-                                                     "%s/%s", state->dirpath,
-                                                     found_name);
-                       } else {
-                               tmp = talloc_strdup(state->smb_fname,
-                                                   found_name);
+                               tmp = unmangled;
                        }
                        if (tmp == NULL) {
                                DBG_ERR("talloc failed\n");
@@ -753,28 +695,84 @@ static NTSTATUS unix_convert_step_stat(struct uc_state *state)
                        }
                        TALLOC_FREE(state->smb_fname->base_name);
                        state->smb_fname->base_name = tmp;
-                       state->name = state->smb_fname->base_name + name_ofs;
+                       state->name =
+                               state->smb_fname->base_name + name_ofs;
+                       state->end = state->name + strlen(state->name);
+               }
 
-                       /*
-                        * We just scanned for, and found the end of
-                        * the path. We must return a valid stat struct
-                        * if it exists. JRA.
-                        */
+               DBG_DEBUG("New file [%s]\n", state->name);
+               state->done = true;
+               return NT_STATUS_OK;
+       }
 
-                       if (state->posix_pathnames) {
-                               ret = SMB_VFS_LSTAT(state->conn, state->smb_fname);
-                       } else {
-                               ret = SMB_VFS_STAT(state->conn, state->smb_fname);
-                       }
 
-                       if (ret != 0) {
-                               SET_STAT_INVALID(state->smb_fname->st);
-                       }
+       /*
+        * Restore the rest of the string. If the string was
+        * mangled the size may have changed.
+        */
+       if (state->end) {
+               char *tmp;
+               size_t name_ofs =
+                       state->name - state->smb_fname->base_name;
+
+               if (!ISDOT(state->dirpath)) {
+                       tmp = talloc_asprintf(state->smb_fname,
+                                             "%s/%s/%s", state->dirpath,
+                                             found_name, state->end+1);
+               }
+               else {
+                       tmp = talloc_asprintf(state->smb_fname,
+                                             "%s/%s", found_name,
+                                             state->end+1);
+               }
+               if (tmp == NULL) {
+                       DBG_ERR("talloc_asprintf failed\n");
+                       return NT_STATUS_NO_MEMORY;
                }
+               TALLOC_FREE(state->smb_fname->base_name);
+               state->smb_fname->base_name = tmp;
+               state->name = state->smb_fname->base_name + name_ofs;
+               state->end = state->name + strlen(found_name);
+               *state->end = '\0';
+       } else {
+               char *tmp;
+               size_t name_ofs =
+                       state->name - state->smb_fname->base_name;
+
+               if (!ISDOT(state->dirpath)) {
+                       tmp = talloc_asprintf(state->smb_fname,
+                                             "%s/%s", state->dirpath,
+                                             found_name);
+               } else {
+                       tmp = talloc_strdup(state->smb_fname,
+                                           found_name);
+               }
+               if (tmp == NULL) {
+                       DBG_ERR("talloc failed\n");
+                       return NT_STATUS_NO_MEMORY;
+               }
+               TALLOC_FREE(state->smb_fname->base_name);
+               state->smb_fname->base_name = tmp;
+               state->name = state->smb_fname->base_name + name_ofs;
+
+               /*
+                * We just scanned for, and found the end of
+                * the path. We must return a valid stat struct
+                * if it exists. JRA.
+                */
 
-               TALLOC_FREE(found_name);
-       } /* end else */
+               if (state->posix_pathnames) {
+                       ret = SMB_VFS_LSTAT(state->conn, state->smb_fname);
+               } else {
+                       ret = SMB_VFS_STAT(state->conn, state->smb_fname);
+               }
+
+               if (ret != 0) {
+                       SET_STAT_INVALID(state->smb_fname->st);
+               }
+       }
 
+       TALLOC_FREE(found_name);
        return NT_STATUS_OK;
 }