Another loop in tdb.c detected

[Jeremy Allison]

On Wed, Jun 15, 2005 at 11:20:09PM -0700, Jeremy Allison wrote:
> 
> The reason is that the || operator evaluates left to right, but it's
> sometimes hard to remember that and I don't want anyone to get confused
> about which order things are done. Also, I'm a little confused as to
> why your previous patch to detect infinite loops didn't catch this.
> 
> The code goes on :
> 
>                 /* Iterate through chain */
>                 while( tlock->off) {
>                         tdb_off current;
>                         if (rec_read(tdb, tlock->off, rec) == -1)
>                                 goto fail;
>                         if (!TDB_DEAD(rec)) {
>                                 /* Woohoo: we found one! */
>                                 if (lock_record(tdb, tlock->off) != 0)
>                                         goto fail;
>                                 return tlock->off;
>                         }
> 
>                         /* Detect infinite loops. From "Shlomi Yaakobovich" <Shlomi at exanet.com>. */
>                         if (tlock->off == rec->next) {
>                                 TDB_LOG((tdb, 0, "tdb_next_lock: loop detected.\n"));
>                                 goto fail;
>                         }
> 
> Shouldn't your "if (tlock->off == rec->next)" check in this
> code segment have caught this already ?

Ok, now I see why that is - wouldn't the following fix catch the
same problem and cover both cases ?

Jeremy.
-------------- next part --------------
Index: tdb/tdb.c
===================================================================
--- tdb/tdb.c	(revision 7627)
+++ tdb/tdb.c	(working copy)
@@ -1270,6 +1270,13 @@
 			tdb_off current;
 			if (rec_read(tdb, tlock->off, rec) == -1)
 				goto fail;
+
+			/* Detect infinite loops. From "Shlomi Yaakobovich" <Shlomi at exanet.com>. */
+			if (tlock->off == rec->next) {
+				TDB_LOG((tdb, 0, "tdb_next_lock: loop detected.\n"));
+				goto fail;
+			}
+
 			if (!TDB_DEAD(rec)) {
 				/* Woohoo: we found one! */
 				if (lock_record(tdb, tlock->off) != 0)
@@ -1277,12 +1284,6 @@
 				return tlock->off;
 			}
 
-			/* Detect infinite loops. From "Shlomi Yaakobovich" <Shlomi at exanet.com>. */
-			if (tlock->off == rec->next) {
-				TDB_LOG((tdb, 0, "tdb_next_lock: loop detected.\n"));
-				goto fail;
-			}
-
 			/* Try to clean dead ones from old traverses */
 			current = tlock->off;
 			tlock->off = rec->next;