From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16287 invoked by alias); 1 Oct 2008 01:23:25 -0000 Received: (qmail 16279 invoked by uid 22791); 1 Oct 2008 01:23:24 -0000 X-Spam-Status: No, hits=-0.4 required=5.0 tests=AWL,BAYES_20,J_CHICKENPOX_74,J_CHICKENPOX_75,J_CHICKENPOX_83,KAM_MX,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 01 Oct 2008 01:22:49 +0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id m911MlBf007063 for ; Tue, 30 Sep 2008 21:22:47 -0400 Received: from pobox-3.corp.redhat.com (pobox-3.corp.redhat.com [10.11.255.67]) by int-mx1.corp.redhat.com (8.13.1/8.13.1) with ESMTP id m911MlHM023650; Tue, 30 Sep 2008 21:22:47 -0400 Received: from touchme.toronto.redhat.com (IDENT:postfix@touchme.yyz.redhat.com [10.15.16.9]) by pobox-3.corp.redhat.com (8.13.1/8.13.1) with ESMTP id m911MUh2005721; Tue, 30 Sep 2008 21:22:47 -0400 Received: from ton.toronto.redhat.com (ton.yyz.redhat.com [10.15.16.15]) by touchme.toronto.redhat.com (Postfix) with ESMTP id 1BFB98001FF; Tue, 30 Sep 2008 21:22:14 -0400 (EDT) Received: from ton.toronto.redhat.com (localhost.localdomain [127.0.0.1]) by ton.toronto.redhat.com (8.13.1/8.13.1) with ESMTP id m911LvEG017988; Tue, 30 Sep 2008 21:21:57 -0400 Received: (from fche@localhost) by ton.toronto.redhat.com (8.13.1/8.13.1/Submit) id m911LvJt017987; Tue, 30 Sep 2008 21:21:57 -0400 X-Authentication-Warning: ton.toronto.redhat.com: fche set sender to fche@redhat.com using -f To: rarora@redhat.com Cc: systemtap@sources.redhat.com Subject: Re: BZ 6701 - Improve error messages (patch) References: <48E29E22.4070708@redhat.com> From: fche@redhat.com (Frank Ch. Eigler) Date: Wed, 01 Oct 2008 01:23:00 -0000 In-Reply-To: <48E29E22.4070708@redhat.com> (Rajan Arora's message of "Tue, 30 Sep 2008 17:46:10 -0400") Message-ID: User-Agent: Gnus/5.1008 (Gnus v5.10.8) Emacs/21.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Scanned-By: MIMEDefang 2.58 on 172.16.52.254 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 X-SW-Source: 2008-q4/txt/msg00000.txt.bz2 Hi, Rajan - > This is a proposed patch for the Bugzilla bug 6701 [...] Thanks, nice work! > # stap -ve 'probe begin { log("hello world") exit () }' > Pass 1: parsed user script and 45 library script(s) in 150usr/10sys/172real ms. > semantic error: unresolved arity-0 function: identifier 'exxit' at :1:34 > source: probe begin { log("hello world") exxit () } > ^ Please check the rendering heuristics for the case of the inputs containing tabs and for lines perhaps too long to draw in their entirety. > + //Do file_contents exist? If not, error is in tapset! > + if (e.tok1 && user_file->file_contents.size() != 0) > + { > + errormsg.str (""); > + print_error_source (errormsg, user_file->file_contents, e.tok1); > + cerr << errormsg.str(); > + } For this and similar cases, is there some reason for the errormsg object? Would this not work just as well? print_error_source (cerr, user_file->file_contents, e.tok1); > +systemtap_session::print_error_source (std::stringstream& message, > + std::string& file_contents, const token* tok) > +{ > [...] > + message << "\tsource: " << file_contents.substr (start_pos, end_pos-start_pos-1) << endl; > + message << "\t "; > + message.fill (' '); > + message.width (col); > + message << "^" << endl; Yeah, this won't work right if the input contains tabs. Instead of the .fill()/.width() way of lining up with the input, you could try looping over the contents of file_contents[start_pos .. end_pos]. You could copy each isspace(char) and emit a ' ' otherwise. > +void > +lexer::get_input_contents (std::string& file_contents) > +{ > + unsigned i; > + for (i=0; i + file_contents += input_contents[i]; > +} Could you look into making that input_contents[] widget into a plain std::string? That should turn this into a simple assignment ... or rather a "return input_contents;" - the function might as well return a std::string instead of a value by reference. > int > lexer::input_peek (unsigned n) > [...] > + //Assuming tapsets are error-free, only user's script contents are fetched > + if (!strstr (input_name.c_str(),"tapset/")) > + input.get_input_contents (f->file_contents); Nothing's error free :-) ... except my wife's mother. Drop this assumption - we can have semantic errors that occur with e.g. $context variables or probe points that exist only on a ragtag collection of kernel versions. > +++ b/testsuite/parseko/source_context.stp > @@ -0,0 +1,11 @@ > +global count_test > +probe timer.ms(123) > +{ > +printf("%d loops have executed\n",count_test) > +count_test ++ > +iffff(count_test == 5) > +{ > +priiintf("Done execution 5 times and about to exit. . .\n") > + eeexit () > +} > +} OK (though one error per file might be more assertive). - FChE