Re: [PATCH/RFC 02/02 v2] Refactor PRPSINFO handling on GDB

Jan Kratochvil
2013-01-11T17:11:59Z

On Fri, 11 Jan 2013 18:03:01 +0100, Sergio Durigan Junior wrote:
> On Friday, January 11 2013, Pedro Alves wrote:
> > On 01/11/2013 03:53 AM, Sergio Durigan Junior wrote:
> >>>> + /* Generating and copying the program's arguments. `get_inferior_args'
> >>>> + may throw, but we want to continue the execution anyway. */
> >>>> + TRY_CATCH (ex, RETURN_MASK_ERROR)
> >>>> + {
> >>>> + infargs = get_inferior_args ();
> >>>> + }
> >>>> +
> >>>
> >>> Hmm? We were not doing that before. What exception is that?
> >>
> >> `get_inferior_args' calls `construct_inferior_arguments', which can call
> >> `error' in an specific scenario (not STARTUP_WITH_SHELL, arguments that
> >> contain spaces).
> >
> > This is an example of something that should be split into
> > its own change, along with its own rationale. This is
> > independent of any refactoring of PRPSINFO handling.
> > We're already calling get_inferior_args nowadays, and I don't
> > ever remember this error being reported as a problem.
>
> My first version of the patch didn't contain the TRY_CATCH part. It was
> Jan who made this suggestion, and I thought it made sense.
>
> I really think a TRY_CATCH does not cause any harm here, but if you
> insist, I can easily remove it from the patch.

I do not think there should be code leaking memory in a case of throwsn
exception when the callee contains an error() call. Even if the error() call
is only in a conditional which with the current setup around can never happen.

I agree one can improve get_inferior_args in a way it no longer throws.
That would be another way to fix it. I do not mind which way the memory leak
of linux_nat_fill_prpsinfo gets fixed.

Thanks,
Jan