Problem in tevent_signal

[simo]

On Wed, 2012-02-08 at 12:15 +0100, Volker Lendecke wrote: 
> On Sun, Jan 29, 2012 at 01:20:30AM +0100, Volker Lendecke wrote:
> > On Sat, Jan 28, 2012 at 04:47:49PM -0500, simo wrote:
> > > Ah sorry I did not see the patch. Yeah it is close but by referncing
> > > itself it becomes hard to read.
> > > 
> > > I was thinking something like this:
> > > 
> > > struct tevent_se_spy {
> > > bool *was_freed;
> > > };
> > > 
> > > static int tevent_se_spy_destructor(struct tevent_se_spy *s)
> > > {
> > > *s->was_freed = true;
> > > }
> > > 
> > > then in tevent_common_check_signal()
> > > 
> > > struct tevent_se_spy *se_spy;
> > > bool se_was_freed = false;
> > > 
> > > se_spy = talloc_zero(se, struct tevent_se_spy);
> > > if (!se_spy) {
> > > continue;
> > > }
> > > se_spy->was_freed = &se_was_freed;
> > > talloc_set_destructor(se_spy, tevent_se_spy_destructor);
> > > 
> > > [...]
> > > 
> > > if (!se_was_freed && (se->sa_flags & SA_RESETHAND)) {
> > > talloc_free(se);
> > > }
> > > 
> > > 
> > > To me the boolean is easier to read and understand, but either way would
> > > work.
> > 
> > I had that first as well. But then I had another
> > if-statement that guarded the talloc_free(exists). So I
> > added the *myself pointer to the structure to null it out.
> > This then led to the boolean go away.
> 
> Here find a new patch that adds a descriptive comment. Maybe
> this can relieve some of your concerns?

Yep, you have a full ACK from me.

Simo.

> Metze, you're the maintainer of tevent. Can you comment on
> this patch?
> 
> Thanks,
> 
> Volker
> 


-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo at samba.org>
Principal Software Engineer at Red Hat, Inc. <simo at redhat.com>