[PATCH] s3: Remove check of constant condition in loop|Resend due to Jeremy's request

Swen Schillig swen at vnet.ibm.com
Mon Mar 19 09:46:49 UTC 2018


On Tue, 2018-03-06 at 15:10 -0800, Jeremy Allison via samba-technical
wrote:
> On Wed, Mar 07, 2018 at 12:05:03AM +0100, Timur I. Bakeyev via samba-
> technical wrote:
> > Hi, Swen!
> > 
> > On 6 March 2018 at 11:46, Swen Schillig via samba-technical <
> > samba-technical at lists.samba.org> wrote:
> > 
> > > Please review ...and push.
> > > 
> > 
> > Not sure which compiler did you use for testing, but Clang isn't
> > happy with
> > nested function declaration:
> 
> Yeah, I think it's a gcc-ism.
> 
> > +	bool (*str_eq)(const char *, const char *) = NULL;
> > +	bool strequal_cs(const char *s1, const char *s2)
> > +		{ return (strcmp(s1, s2) == 0); }
> > +
> > +	str_eq = (conn->case_sensitive) ? strequal_cs : strequal;
> > 
> > 
> > And just refuses to compile. I had to move strequal_cs() outside
> > the
> > function and declare it static:
> > 
> > + static bool strequal_cs(const char *s1, const char *s2)
> > + {
> > +         return (strcmp(s1, s2) == 0);
> > + }
> > 
> > Please, consider such a change.
> 
> +1 - this can't go in as-is, as we need to be portable to
> more than just gcc.
> 
> Jeremy.

Hi Jeremy

here's the 2nd (updated) patch.

Please review and push if OK.

Thanks in advance.

Cheers Swen.
-------------- next part --------------
From 28252cd1e68f2321a518008b3ddf51f5bf72bc15 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Tue, 6 Mar 2018 08:43:46 +0100
Subject: [PATCH] s3: Remove check of constant condition in loop

The function SearchDir contains three loops which check,
for each iteration, whether a string comparison is to be performed
case sesitive or not.
But this condition does not change between loop iterations.
Therefore, introduce a function pointer which is set at the beginning
according to the case sesitivity setting and executes the correct
string comparison in the succeeding code paths.

Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
---
 source3/smbd/dir.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index 8820a117457..23ab563e99b 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -1951,6 +1951,11 @@ static void DirCacheAdd(struct smb_Dir *dirp, const char *name, long offset)
 	e->offset = offset;
 }
 
+static bool strequal_cs(const char *s1, const char *s2)
+{
+	return (strcmp(s1, s2) == 0);
+}
+
 /*******************************************************************
  Find an entry by name. Leave us at the offset after it.
  Don't check for veto or invisible files.
@@ -1962,12 +1967,15 @@ bool SearchDir(struct smb_Dir *dirp, const char *name, long *poffset)
 	const char *entry = NULL;
 	char *talloced = NULL;
 	connection_struct *conn = dirp->conn;
+	bool (*str_eq)(const char *, const char *) = NULL;
+
+	str_eq = (conn->case_sensitive) ? strequal_cs : strequal;
 
 	/* Search back in the name cache. */
 	if (dirp->name_cache_size && dirp->name_cache) {
 		for (i = dirp->name_cache_index; i >= 0; i--) {
 			struct name_cache_entry *e = &dirp->name_cache[i];
-			if (e->name && (conn->case_sensitive ? (strcmp(e->name, name) == 0) : strequal(e->name, name))) {
+			if (e->name && str_eq(e->name, name)) {
 				*poffset = e->offset;
 				SeekDir(dirp, e->offset);
 				return True;
@@ -1975,7 +1983,7 @@ bool SearchDir(struct smb_Dir *dirp, const char *name, long *poffset)
 		}
 		for (i = dirp->name_cache_size - 1; i > dirp->name_cache_index; i--) {
 			struct name_cache_entry *e = &dirp->name_cache[i];
-			if (e->name && (conn->case_sensitive ? (strcmp(e->name, name) == 0) : strequal(e->name, name))) {
+			if (e->name && str_eq(e->name, name)) {
 				*poffset = e->offset;
 				SeekDir(dirp, e->offset);
 				return True;
@@ -1988,7 +1996,7 @@ bool SearchDir(struct smb_Dir *dirp, const char *name, long *poffset)
 	dirp->file_number = 0;
 	*poffset = START_OF_DIRECTORY_OFFSET;
 	while ((entry = ReadDirName(dirp, poffset, NULL, &talloced))) {
-		if (conn->case_sensitive ? (strcmp(entry, name) == 0) : strequal(entry, name)) {
+		if (str_eq(entry, name)) {
 			TALLOC_FREE(talloced);
 			return True;
 		}
-- 
2.14.3



More information about the samba-technical mailing list