[darcs-devel] Better error reporting for exec_ and friends

Magnus Jonsson magnus at smartelectronix.com
Thu Sep 28 21:39:21 PDT 2006


On Fri, 29 Sep 2006, Eric Y. Kow wrote:

> Hi,
>
> On Sun, Sep 24, 2006 at 22:34:54 -0400, Magnus Jonsson wrote:
>> What this patch does is, it introduces a new exception ExecFailed that
>> gets thrown if exec_ fails to at all execute the file. Because of the
>> design of unix, detectin this condition requires some hackery involving
>> pipes (njs suggested this to me on irc). I also added a handler at
>> toplevel main to report the ExecFailed exceptions to the user. Other than
>> this, exec_ and friends behave exactly as before.
>>
>> Let me know what you think. I'm ready to do some revision if you guys
>> think it's needed.
>
> The exec stuff has changed considerably (in the current batch of
> accepted patches).  Would you please rewrite your patch in light of the
> new changes?

Yes, I'll have a look at the new changes and add my changes if needed.

> I haven't had a deep look.  Some questions:
>
> 1) It adds some new exit codes for internal usage, paraphrasing the comments,
>      -1 for fork() failure
>      -2 for pipe(),
>      -3 execvp_no_vtalarm()
>    Dut does this mean we're counting on the programs themselves never
>    exiting with these codes?

exec_extern returns process id of the newly launched child process, or one 
of those error codes if it fails. There used to be no/little error 
checking here.

You then use smart_wait_pid to get the return code, just like before. This
is the same as before.

> 2) These exit codes don't actually get returned by darcs, right? You
>   catch them and then just throw an ExitFailure if it's one of the
>   expected codes

ExecFailure only represents failure to start an external program. It 
doesn't carry with it any exit code, because no program was run.

> 3) I'm a bit nervous about catching ExitFailure at main - it seems like
>   this is something we would want to bubble up.  How do you know this
>   doesn't misreport ExitFailures thrown by darcs itself?

It is better than the current status I think - right now these failures 
pass unseen and if here is a failure, the desired side effects never take 
place. This way darcs realizes that there's a problem and bails out. Darcs
itself shouldn't throw any ExecFailure. Only Exec.hs should throw 
ExecFailure.

> 4) Any reason the bulk of the code is written in C and not Haskell?

I started out writing it in Haskell, but then I realized I would need a 
lot of FFI imports so I decided it would be more straight-forward to do it 
directly in C and expose a minimal interface through the FFI. If you think 
the C code is messy, that's not because it's written in C, but because of 
all the hoops you need to go through to create a process and error-check 
everything.

Best,
Magnus Jonsson




More information about the darcs-devel mailing list