From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12100 invoked by alias); 17 Jul 2012 18:11:25 -0000 Received: (qmail 12091 invoked by uid 22791); 17 Jul 2012 18:11:23 -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,SPF_HELO_PASS,T_RP_MATCHES_RCVD 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; Tue, 17 Jul 2012 18:11:10 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q6HIBAr5004306 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 17 Jul 2012 14:11:10 -0400 Received: from psique (ovpn-113-52.phx2.redhat.com [10.3.113.52]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q6HIB8u2027516 for ; Tue, 17 Jul 2012 14:11:09 -0400 From: Sergio Durigan Junior To: gdb-patches@sourceware.org Subject: Re: [RFA 4/4] Improved linker-debugger interface References: <20120712123406.GA29236@redhat.com> <20120712123617.GE29236@redhat.com> <20120713094150.GB2689@redhat.com> X-URL: http://www.redhat.com Date: Tue, 17 Jul 2012 18:11:00 -0000 In-Reply-To: <20120713094150.GB2689@redhat.com> (Gary Benson's message of "Fri, 13 Jul 2012 10:41:50 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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: 2012-07/txt/msg00238.txt.bz2 On Friday, July 13 2012, Gary Benson wrote: > diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c > index c111f04..ebb8c3c 100644 > --- a/gdb/solib-svr4.c > +++ b/gdb/solib-svr4.c > +/* Decide what action to take when the specified solib event probe is > + hit. */ > + > +static enum probe_action > +solib_event_probe_action (struct probe_and_info *pi) > +{ > + enum probe_action action; > + int update; > + struct obj_section *os; > + unsigned probe_argc; > + struct svr4_info *info; > + CORE_ADDR debug_base; > + > + action = pi->info->action; > + if (action == LM_CACHE_NO_ACTION || action == LM_CACHE_INVALIDATE) > + return action; > + > + gdb_assert (action == LM_CACHE_RELOAD > + || action == LM_CACHE_UPDATE_OR_RELOAD); > + > + os = find_pc_section (pi->probe->address); > + if (os == NULL) > + return LM_CACHE_INVALIDATE; > + > + /* Check that an appropriate number of arguments has been supplied. > + We expect: > + arg1: Lmid_t lmid (mandatory) > + arg2: struct r_debug *r_debug (mandatory) > + arg3: struct link_map *new (optional, for incremental updates) */ I guess you could rename the arguments listed here to 'arg0', 'arg1' and 'arg2', because `evaluate_probe_argument' takes these numbers as arguments. Or you could explicitly say that here. Otherwise it will confuse the reader, IMO. > + probe_argc = get_probe_argument_count (os->objfile, pi->probe); > + if (probe_argc == 2) > + action = LM_CACHE_RELOAD; > + else if (probe_argc < 2) > + return LM_CACHE_INVALIDATE; This is OK... > + /* We only currently support the global namespace (PR gdb/11839). > + If the probe's r_debug doesn't match the global r_debug then > + this event refers to some other namespace and must be ignored. */ > + info = get_svr4_info (); > + > + /* Always locate the debug struct, in case it has moved. */ > + info->debug_base = 0; > + locate_base (info); > + > + debug_base = value_as_address (evaluate_probe_argument (os->objfile, > + pi->probe, 1)); ...but what would happen if `evaluate_probe_argument' returned NULL? It's better to check this, because `value_as_address' calls `value_type' which does not check NULL pointers. Currently, only the SystemTap backend is implemented, and if it returns NULL in this case it would be an error, but it's better to guard your code IMO. Thanks, -- Sergio