This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: BZ 6701 - Improve error messages (patch)


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 <input>: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<input_contents.size(); 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]