Samba caseinsensitive lookup benchmark

[James Peach]

On 11/04/2008, at 2:45 PM, Jeremy Allison wrote:
> On Fri, Apr 11, 2008 at 02:45:15PM -0700, James Peach wrote:
>>
>> I'm talking about the benchmark patch, not the case-insensitive  
>> filesystem
>> fix. The latter is already review and released. If you want it, I  
>> can merge
>> it any time.
>
> Ok, thanks. Yes I'd like you to merge it for 3.2, but only after
> we've agreed on the change.
>
>> They are different things. fs_capabilities tells you the semantics  
>> that the
>> filesystem is providing to Samba. The (conn)->case_sensitive flag  
>> tells you
>> the semantics that the Samba is providing to the SMB client. I'm  
>> not sure
>> wether this was the original intent of the latter flag, but that  
>> seems to
>> be is implementation.
>
> I know they're different things, and are set in different places.
> I'm not objecting to fs_capabilities being added to the connection
> struct, indeed it is a useful addition. What I mean is everywhere
> we currently change behavior depending on case insensitivity we
> should be checking both the fs_capabilities flags and the  
> case_sensitive
> bool and should be doing the same thing for both. The *behavior*
> should be identical. That's what I meant with my macro below.
>
>>> FS_IS_CASE_INSENSITIVE(conn) (((conn)->fs_capabilities &
>>> FILE_CASE_SENSITIVE_SEARCH) || (!(conn)->case_sensitive))
>>

The XFS folks will need to figure out some way to communicate the  
filesystem capabilities to Samba.

>> Yeh, that looks reasonable.
>
> Thanks. Any chance you can update the patch including it
> for review ?

errr ... looks like I already pushed this patch a while ago.


Here's the cleanup you suggest. I didn't alter the test in  
unix_convert() because it's testing !(conn->case_sensitive), rather  
than the reverse.


diff --git a/source/include/smb.h b/source/include/smb.h
index d52d849..da60daf 100644
--- a/source/include/smb.h
+++ b/source/include/smb.h
@@ -674,6 +674,16 @@ typedef struct connection_struct {
  	struct notify_context *notify_ctx;
  } connection_struct;

+/* True if the underlying filesystem for this connection is case- 
insensitive. */
+#define FS_IS_CASE_INSENSITIVE(conn) \
+	(! ((conn)->fs_capabilities & FILE_CASE_SENSITIVE_SEARCH) )
+
+/* True if we should avoid doing case-matching filename searches on  
this
+ * connection.
+ */
+#define NO_CASE_MATCHING_SEARCH(conn) \
+    ( FS_IS_CASE_INSENSITIVE(conn) || ((conn)->case_sensitive) )
+
  struct current_user {
  	connection_struct *conn;
  	uint16 vuid;
diff --git a/source/smbd/dir.c b/source/smbd/dir.c
index 6e02401..e207ddc 100644
--- a/source/smbd/dir.c
+++ b/source/smbd/dir.c
@@ -651,8 +651,7 @@ const char *dptr_ReadDirName(TALLOC_CTX *ctx,
  		 * filesystem is case sensitive.
  		 */

-		if (dptr->conn->case_sensitive ||
-		    !(dptr->conn->fs_capabilities & FILE_CASE_SENSITIVE_SEARCH)) {
+		if (NO_CASE_MATCHING_SEARCH(dptr->conn)) {
  			/* We need to set the underlying dir_hnd offset to -1 also as
  			   this function is usually called with the output from TellDir. */
  			dptr->dir_hnd->offset = *poffset = END_OF_DIRECTORY_OFFSET;
diff --git a/source/smbd/filename.c b/source/smbd/filename.c
index 4323e84..280edc1 100644
--- a/source/smbd/filename.c
+++ b/source/smbd/filename.c
@@ -250,7 +250,7 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
  	 * authoratitive. JRA.
  	 */

-	if((!conn->case_sensitive || !(conn->fs_capabilities &  
FILE_CASE_SENSITIVE_SEARCH)) &&
+	if((!conn->case_sensitive || FS_IS_CASE_INSENSITIVE(conn)) &&
  			stat_cache_lookup(conn, &name, &dirpath, &start, &st)) {
  		*pst = st;
  		goto done;
@@ -301,7 +301,7 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
  	 * won't help.
  	 */

-	if ((conn->case_sensitive || !(conn->fs_capabilities &  
FILE_CASE_SENSITIVE_SEARCH)) &&
+	if ((conn->case_sensitive || FS_IS_CASE_INSENSITIVE(conn)) &&
  			!mangle_is_mangled(name, conn->params)) {
  		goto done;
  	}
@@ -790,7 +790,7 @@ static bool scan_directory(connection_struct  
*conn, const char *path,
  	 * good to search for a name. If a case variation of the name was
  	 * there, then the original stat(2) would have found it.
  	 */
-	if (!mangled && !(conn->fs_capabilities &  
FILE_CASE_SENSITIVE_SEARCH)) {
+	if (!mangled && NO_CASE_MATCHING_SEARCH(conn)) {
  		errno = ENOENT;
  		return False;
  	}