Issue with changes to VFS_STAT and VFS_LSTAT.
Jeremy Allison
jra at samba.org
Fri Jun 26 16:47:34 MDT 2009
Tim,
I need some clarifications on the changes you made in:
36c10191750c845a2a7cd6cc62149b1095c0b651..e129384d7c1df664e447186673dd107e190e2894
(s3: Plumb smb_filename through SMB_VFS_STAT and SMB_VFS_LSTAT
This patch introduces two new temporary helper functions
vfs_stat_smb_fname and vfs_lstat_smb_fname. They basically allowed me
to call the new smb_filename version of stat, while avoiding plumbing
it through callers that are still too inconvenient. As the conversion
moves along, I will be able to remove callers of this, with the goal
being to remove all callers.)
This pushes the 'struct smb_filename' into visibility at the
POSIX VFS layer, which as a shorthand to keep the stat struct
tied to the filename is fine.
However, in the default stat() implementation it now calls :
get_full_smb_filename() which concatinates the base name + stream
name (if one exists). Which means potentially the full stream
names are visible to an underlying POSIX system than cannot
cope with them.
Is this desired ? The logic in the upper level smbd should
ensure that basic POSIX stat is containing a stream name is never
called if the underlying filesystem can't support them.
My feeling is in the "default" module case which remember is
the last stop before hitting a *pure* "POSIX" filesystem we should
do an SMB_ASSERT() that smb_fname->stream_name == NULL, as I can't
see any case where the "default" POSIX filesystem can do
anything but error out here. It actually is exposing a logic error
in the upper level logic IMHO. A simple patch for this follows.
We also need to clarify the comments around this in all
the modules to help third-party VFS implementors to understand
*exactly* what should be going on here.
Same for the LSTAT case I think.
Can you get back to me on this and let me know what you
think ?
Jeremy.
-------------- next part --------------
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index cd792ab..1c5aa5e 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -553,21 +553,11 @@ static int vfswrap_stat(vfs_handle_struct *handle,
struct smb_filename *smb_fname)
{
int result;
- NTSTATUS status;
- char *fname = NULL;
START_PROFILE(syscall_stat);
- status = get_full_smb_filename(talloc_tos(), smb_fname,
- &fname);
- if (!NT_STATUS_IS_OK(status)) {
- errno = map_errno_from_nt_status(status);
- return -1;
- }
-
- result = sys_stat(fname, &smb_fname->st);
-
- TALLOC_FREE(fname);
+ SMB_ASSERT(smb_fname->stream_name == NULL);
+ result = sys_stat(smb_fname->base_name, &smb_fname->st);
END_PROFILE(syscall_stat);
return result;
@@ -587,21 +577,11 @@ static int vfswrap_lstat(vfs_handle_struct *handle,
struct smb_filename *smb_fname)
{
int result;
- NTSTATUS status;
- char *fname = NULL;
START_PROFILE(syscall_lstat);
- status = get_full_smb_filename(talloc_tos(), smb_fname,
- &fname);
- if (!NT_STATUS_IS_OK(status)) {
- errno = map_errno_from_nt_status(status);
- return -1;
- }
-
- result = sys_lstat(fname, &smb_fname->st);
-
- TALLOC_FREE(fname);
+ SMB_ASSERT(smb_fname->stream_name == NULL);
+ result = sys_lstat(smb_fname->base_name, &smb_fname->st);
END_PROFILE(syscall_lstat);
return result;
More information about the samba-technical
mailing list