On Tue, 19 Nov 2024 10:49:39 +0100 Corinna Vinschen wrote: > On Nov 19 17:39, Takashi Yano wrote: > > On Mon, 18 Nov 2024 16:55:14 +0100 > > Corinna Vinschen wrote: > > > On Nov 15 22:14, Takashi Yano wrote: > > > > The commit ae181b0ff122 has a bug that the pointer is referred bofore > > > > NULL check in the function lf_clearlock(). This patch fixes that. > > > > > > > > Addresses: https://cygwin.com/pipermail/cygwin/2024-November/256750.html > > > > Fixes: ae181b0ff122 ("Cygwin: lockf: Make lockf() return ENOLCK when > > > > too many locks") > > > > Reported-by: Sebastian Feld <sebastian.n.f...@gmail.com> > > > > Reviewed-by: > > > > Signed-off-by: Takashi Yano <takashi.y...@nifty.ne.jp> > > > > --- > > > > winsup/cygwin/flock.cc | 6 ++++-- > > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/winsup/cygwin/flock.cc b/winsup/cygwin/flock.cc > > > > index 3821bddd6..794e66bd7 100644 > > > > --- a/winsup/cygwin/flock.cc > > > > +++ b/winsup/cygwin/flock.cc > > > > @@ -1524,6 +1524,10 @@ lf_clearlock (lockf_t *unlock, lockf_t **clean, > > > > HANDLE fhdl) > > > > lockf_t *lf = *head; > > > > lockf_t *overlap, **prev; > > > > int ovcase; > > > > + > > > > + if (lf == NOLOCKF) > > > > + return 0; > > > > + > > > > inode_t *node = lf->lf_inode; > > > > tmp_pathbuf tp; > > > > node->i_all_lf = (lockf_t *) tp.w_get (); > > > > @@ -1531,8 +1535,6 @@ lf_clearlock (lockf_t *unlock, lockf_t **clean, > > > > HANDLE fhdl) > > > > uint32_t lock_cnt = node->get_lock_count (); > > > > bool first_loop = true; > > > > > > > > - if (lf == NOLOCKF) > > > > - return 0; > > > > prev = head; > > > > while ((ovcase = lf_findoverlap (lf, unlock, SELF, &prev, &overlap))) > > > > { > > > > -- > > > > 2.45.1 > > > > > > LGTM, please push. > > > > Thanks for reviewing this patch. Could you please review > > [PATCH v2] Cygwin: flock: Fix overlap handling in lf_setlock() and > > lf_clearlock() > > as well? > > Give me a bit of time. While the patch might fix the problem, what > bugs me is the deviation from upstream code. We will at least need > a few comments to explain why we don't follow the upstream behaviour.
I've got it. Does this code come from 'upstream'? From what code? Essentially, the ovcase 1 can be a part of ovcase 3. I guess the 'upstream' does not add lock entry having same lock range unlike current cygwin (lf_ver related). So, ovcase 1 can break after handling 1 overlap. However, we need find overlap repeatedly because we have lf_ver. -- Takashi Yano <takashi.y...@nifty.ne.jp>
