[PATCH 2/2] Cygwin: lockf: Make lockf() return ENOLCK when too many locks

[Takashi Yano]

On Tue, 22 Oct 2024 18:02:00 +0200
Corinna Vinschen <corinna-cygwin@cygwin.com> wrote:
> > @@ -503,7 +506,8 @@ inode_t::get (dev_t dev, ino_t ino, bool create_if_missing, bool lock)
> >  }
> >  
> >  inode_t::inode_t (dev_t dev, ino_t ino)
> > -: i_lockf (NULL), i_all_lf (NULL), i_dev (dev), i_ino (ino), i_cnt (0L)
> > +: i_lockf (NULL), i_all_lf (NULL), i_dev (dev), i_ino (ino), i_cnt (0L),
> > +  i_lock_cnt (0)
> >  {
> >    HANDLE parent_dir;
> >    WCHAR name[48];
> > @@ -610,17 +614,15 @@ inode_t::get_all_locks_list ()
> >  	  dbi->ObjectName.Buffer[LOCK_OBJ_NAME_LEN] = L'\0';
> >  	  if (!newlock.from_obj_name (this, &i_all_lf, dbi->ObjectName.Buffer))
> >  	    continue;
> > -	  if (lock - i_all_lf >= MAX_LOCKF_CNT)
> > -	    {
> > -	      system_printf ("Warning, can't handle more than %d locks per file.",
> > -			     MAX_LOCKF_CNT);
> > -	      break;
> > -	    }
> > +	  if (lock - i_all_lf > MAX_LOCKF_CNT)
> > +	    api_fatal ("Can't handle more than %d locks per file.",
> > +		       MAX_LOCKF_CNT);
> 
> I don't quite get that. The commit message says to return ENOLCK rather
> than printing a warning, but here you call api_fatal(), which is even
> more extrem?  Did I miss something?

Yeah, this should not happen. So assert() might be better approach.
Please review again for v2 patch. Problems in this patch also has
been fixed.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>