From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6054 invoked by alias); 26 Jun 2013 22:40:35 -0000 Mailing-List: contact systemtap-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: systemtap-owner@sourceware.org Received: (qmail 5986 invoked by uid 89); 26 Jun 2013 22:40:30 -0000 X-Spam-SWARE-Status: No, score=-7.6 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Wed, 26 Jun 2013 22:40:30 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r5QMeRoL021686 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 26 Jun 2013 18:40:28 -0400 Received: from [10.3.113.36] (ovpn-113-36.phx2.redhat.com [10.3.113.36]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r5QMeRIO024327; Wed, 26 Jun 2013 18:40:27 -0400 Message-ID: <51CB6DDB.7080203@redhat.com> Date: Wed, 26 Jun 2013 22:40:00 -0000 From: Josh Stone User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130612 Thunderbird/17.0.6 MIME-Version: 1.0 To: "Yichun Zhang (agentzh)" CC: systemtap@sourceware.org Subject: Re: [PATCH v3 1/1] PR11096: Add support for the "module" argument to @var References: <51C8EF57.2050801@redhat.com> <1372225343-3618-1-git-send-email-agentzh@gmail.com> <1372225343-3618-2-git-send-email-agentzh@gmail.com> In-Reply-To: <1372225343-3618-2-git-send-email-agentzh@gmail.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-q2/txt/msg00384.txt.bz2 I have just two small comments, which you can probably just fix without further review, then I think this is ready to push! > void > +dwarf_var_expanding_visitor::visit_atvar_op (atvar_op *e) > +{ > + if (e->module.empty() && e->cu_name.empty()) > + { > + // Fill in the module name so that even if there is no match among > + // local variables, dwarf_atvar_expanding_visitor can still scan > + // all the CUs in the current module for a global variable match. > + e->module = q.dw.module_name; Add a similar e->module update in sdt_uprobe_var_expanding_visitor. That's superfluous when an sdt module does have debuginfo, since it will end up in this dwarf visitor eventually. But SDT can technically get resolved without debuginfo, and we don't want any @var from this context to default to "kernel" in that case. (It will still fail without debuginfo, but it should fail with a correct message.) > + /* Unable to find the variable in the current module, so we chain > + * an error in atvar_op */ > + semantic_error er(_F("unable to find global '%s' in %s, %s %s", > + e->sym_name().c_str(), module.c_str(), > + e->cu_name.empty() ? "" : _("in"), > + e->cu_name.c_str())); This message looks awkward for the empty-cu case, e.g. [...]unable to find global 'foo' in kernel, : operator '@var'[...] I suggest changing "%s, %s %s" to "%s%s%s", and "in" to ", in ". Thanks, Josh