On Tue, 3 Dec 2024 21:17:47 +0900
Takashi Yano wrote:
> On Mon, 2 Dec 2024 16:14:59 +0100
> Corinna Vinschen wrote:
> > On Nov 29 20:48, Takashi Yano wrote:
> > > The queue is cleaned up by removing the entries having si_signo == 0
> > > while processing the queued signals, however, sipacket::process() may
> > > set si_signo in the queue to 0 of the entry already processed but not
> > > succeed by calling sig_clear(). This patch ensures the sig_clear()
> > > to remove the entry from the queue chain.
> > 
> > Thanks for this patch.  I just have two questions.
> > 
> > > Addresses: https://cygwin.com/pipermail/cygwin/2024-November/256744.html
> > > Fixes: 9d2155089e87 ("(wait_sig): Define variable q to be the start of 
> > > the signal queue.  Just iterate through sigq queue, deleting processed or 
> > > zeroed signals")
> > > Reported-by: Christian Franke <christian.fra...@t-online.de>
> > > Reviewed-by: Corinna Vinschen <cori...@vinschen.de>
> > > Signed-off-by: Takashi Yano <takashi.y...@nifty.ne.jp>
> > > ---
> > >  winsup/cygwin/local_includes/sigproc.h |  3 +-
> > >  winsup/cygwin/sigproc.cc               | 48 +++++++++++++++++---------
> > >  2 files changed, 34 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/winsup/cygwin/local_includes/sigproc.h 
> > > b/winsup/cygwin/local_includes/sigproc.h
> > > index 46e26db19..8b7062aae 100644
> > > --- a/winsup/cygwin/local_includes/sigproc.h
> > > +++ b/winsup/cygwin/local_includes/sigproc.h
> > > @@ -50,8 +50,9 @@ struct sigpacket
> > >    {
> > >      HANDLE wakeup;
> > >      HANDLE thread_handle;
> > > -    struct sigpacket *next;
> > >    };
> > > +  struct sigpacket *next;
> > > +  struct sigpacket *prev;
> > 
> > The former method using q and qnext ptr didn't need prev.  The question
> > is, why did you add prev?  If you think this has an advantage, even if
> > just better readability, it would be nice to document this in the commit
> > message.
> 
> Consider the queued signal chain is like:
> A->B->C->D
> Assume that now 'q' and 'qnext' ptr points to C and process() is
> processing C. If B is cleared in process(), A->next should be set
> to C.
in clear().

> Then, if process() for C succeeds, C should be removed from
> the queue, so A->next should be set to D. However, now we cannot
> access to A because we do not have the pointer to A.
in while loop in wait_sig().

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

Reply via email to