Patch/Design communication (Re: frysk-core/frysk util/TestFStack.java util/Sta ...)

Patch/Design communication (Re: frysk-core/frysk util/TestFStack.java util/Sta ...)

[Mark Wielaard]

Hi,

On Fri, 2006-12-15 at 20:51 +0000, npremji@sourceware.org wrote:
> Log message:
> 	frysk-core/frysk/bindir/CL
> 	* fcore.java: Reverted ProcException addition.
> 	* fstack.java: Ditto.
> 	frysk-core/frysk/util/CL
> 	* StacktraceAction.java: Revert ProcException.
> 	* CoredumpAction.java: Ditto.
> 	* TestFStack.java: Ditto.
> 	* TestFCore.java: Ditto.
> 	frysk-core/frysk/proc/CL
> 	* ProcException.java: Removed.
> 	* ProcBlockAction.java: Reverted ProcException.
> 	* TestProcForceDetach: Ditto.
> 	* TestProcStopped: Ditto.

What was the intention of the addition and subsequent removal of this?
If I understood the code correctly we could be handed a null proc or a
proc that we don't control/own. Where/How is this dealt with now?

This isn't specific to this patch btw. I hope that we can in general
discuss more of the design decisions on the main mailinglist. I try to
read all commit messages, but just going over each patch does not always
give me insight into the higher level debate that must have gone on for
some of the changed.

Cheers,

Mark

Re: Patch/Design communication (Re: frysk-core/frysk util/TestFStack.java util/Sta ...)

[Nurdin Premji]

On Mon, 2006-12-18 at 12:24 +0100, Mark Wielaard wrote:
> Hi,
> 
> On Fri, 2006-12-15 at 20:51 +0000, npremji@sourceware.org wrote:
> > Log message:
> > 	frysk-core/frysk/bindir/CL
> > 	* fcore.java: Reverted ProcException addition.
> > 	* fstack.java: Ditto.
> > 	frysk-core/frysk/util/CL
> > 	* StacktraceAction.java: Revert ProcException.
> > 	* CoredumpAction.java: Ditto.
> > 	* TestFStack.java: Ditto.
> > 	* TestFCore.java: Ditto.
> > 	frysk-core/frysk/proc/CL
> > 	* ProcException.java: Removed.
> > 	* ProcBlockAction.java: Reverted ProcException.
> > 	* TestProcForceDetach: Ditto.
> > 	* TestProcStopped: Ditto.
> 
> What was the intention of the addition and subsequent removal of this?
> If I understood the code correctly we could be handed a null proc or a
> proc that we don't control/own. Where/How is this dealt with now?
> 
> This isn't specific to this patch btw. I hope that we can in general
> discuss more of the design decisions on the main mailinglist. I try to
> read all commit messages, but just going over each patch does not always
> give me insight into the higher level debate that must have gone on for
> some of the changed.
> 
> Cheers,
> 
> Mark
> 
The exception stuff was added to handle cases where a ProcBlockAction
could not be performed, such as a null process or restrictive
permissions.
The exception stuff was removed because it causes problems with program
flow. So there was a discussion on irc this morning about what the best
way to go about sending warnings/errors back to the user was.

<cagney> npremji, the problem with throwing an exception is: in general
it makes code flow more complicated - it had better be exceptional; and
for a multi-threaded program the exception is invariably thrown in the
wrong thread
<cagney> npremji, in what ways can the ProcBlock object fail?
<npremji> cagney, well the only two things that were checked were a null
process (I think that was vintage refreshAll code) and proc ownership.
<cagney> npremji, ok, for NULL, the code is broken and should be allowed
to crash and burn
<npremji> cagney, yup
<cagney> npremji, that just leaves an attempt to add failing?
<cagney> npremji, in general it isn't possible to check before hand that
an attach will fail due to permission
<npremji> cagney, yeah I guess that is right, I can send a call to
addFailed.
<cagney> npremji, this is because there's a race between a process
alterning its permission or execing something that does, and frysk
trying to do the attach
<cagney> npremji, right, more explicit and much more straight forward
<npremji> cool.