Another loop in tdb.c detected

[Shlomi Yaakobovich]

Hi,

Yeah, I think that it is indeed a better solution, just do the check in one place, before returning from the function, and before the next iteration.

Shlomi

> -----Original Message-----
> From: Jeremy Allison [mailto:jra at samba.org]
> Sent: Thursday, June 16, 2005 09:47
> To: Jeremy Allison
> Cc: Shlomi Yaakobovich; samba-technical at lists.samba.org
> Subject: Re: Another loop in tdb.c detected
> 
> 
> 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.
>