From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25873 invoked by alias); 11 Jan 2013 17:11:59 -0000 Received: (qmail 25854 invoked by uid 22791); 11 Jan 2013 17:11:58 -0000 X-SWARE-Spam-Status: No, hits=-6.3 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 11 Jan 2013 17:11:51 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r0BHBoSw007093 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 11 Jan 2013 12:11:50 -0500 Received: from host2.jankratochvil.net (ovpn-116-19.ams2.redhat.com [10.36.116.19]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r0BHBkuv011113 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Fri, 11 Jan 2013 12:11:48 -0500 Date: Fri, 11 Jan 2013 17:11:00 -0000 From: Jan Kratochvil To: Sergio Durigan Junior Cc: Pedro Alves , GDB Patches , Binutils Development , "H.J. Lu" Subject: Re: [PATCH/RFC 02/02 v2] Refactor PRPSINFO handling on GDB Message-ID: <20130111171145.GA26852@host2.jankratochvil.net> References: <20121218171555.GA19639@host2.jankratochvil.net> <20121231194134.GA17955@host2.jankratochvil.net> <50EF0BE3.6040503@redhat.com> <50F02661.8020400@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2013-01/txt/msg00236.txt.bz2 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