Re: How to write a proper fork hook code?

[Svante Signell]

On Mon, 2015-03-02 at 10:38 +0100, Samuel Thibault wrote:
> Svante Signell, le Mon 02 Mar 2015 10:17:52 +0100, a écrit :
> > On Mon, 2015-03-02 at 10:09 +0100, Samuel Thibault wrote:
> > > Svante Signell, le Mon 02 Mar 2015 08:29:55 +0100, a écrit :
> > > > Well the basic problem is of course to detect processes generated by
> > 
> > >  There's no hidden
> > > process in there, and the fork hook should get you done.
> > 
> > I never talked about any hidden process, did I?
> 
> Then why would you need to detect them?  You have the parent and child
> hooks, they are working.

So you mean that I should call the unlock hook code in the parent?? That
would unlock the parent locks not the child locks. The child hook code
does not work as written, aswe now have found out. fork(2) works if an
FD is created or not, see fork.c code below.

> > > But again, I'm still unsure why you believe that locks are being
> > > inherited through fork(): before introducing a fork hook to forcibly
> > > unlock something, one needs to be absolutely sure that it's actually
> > > locked.  Actually it'd even look like a design flaw if two process could
> > > actually hold a lock on the same piece of a file...
> > 
> > Well, the patches are already submitted, and there is test code for
> > locks inherited by fork(2): fork.c
> 
> You mean test_fork.c?  As I've mentioned earlier:

No, I mean fork.c written by Neal.
http://lists.gnu.org/archive/html/bug-hurd/2015-01/msg00104.html

> «
> I believe there is a misunderstanding here: F_GETLK does not
> return whether the caller has a lock on the file, but whether there is
> an existing lock that would prevent from acquiring one.
> »
> So I don't believe it is actually testing for inherited locks.
> 

As I wrote, now stating it in clear text: it is testing to see if a lock
can be placed. And don't bother about that code, it is the RPC version
that is of interest (or fork.c, see above).

> > Maybe you should apply the patches
> > and see for yourself, instead of always doubt everything I report :(
> 
> Well, you have to understand that review precisely *is* about doubting.
> Writing correct code is about doubting as much as possible to avoid
> bugs.  If I'm not convinced that the program works one way or another,
> I can't apply the patch.  Either I get convinced by reading the source
> code and run tests, which would take time for me, and is thus unlikely
> to happen soon; or you explain why you think it is working one way
> or another.  What I have seen so far doesn't convince me one way or
> another.  I'm sorry, but that's how opensource works, you can't just
> push code without explanations.

I'll given every explanation you asked for so far. Anything else you
want to know?