[Ltrace-devel] [RFC 4/5] Maintain a 1-to-1 mapping between func params and arguments

Edgar E. Iglesias edgar.iglesias at gmail.com
Mon Oct 15 08:40:39 UTC 2012


On Sun, Oct 14, 2012 at 12:31:04AM +0200, Petr Machata wrote:
> edgar.iglesias at gmail.com writes:
> 
> > From: "Edgar E. Iglesias" <edgar at axis.com>
> >
> > When mapping function parameters into argument value dictionaries,
> > make sure to keep a 1-to-1 mapping between indexes.
> >
> > For STOP parameters, we insert a ARGTYPE_NONE value to fill out
> > the argument value dictionary.
> >
> > Mainting correct is important because the various param expressions
> > refer to eachother based on indexes.
> 
> I had a chance to take a look today.  Your fix breaks arg* references in
> configuration files, because + becomes a numbered parameter:
> 
> $ ./ltrace -a0 -efunc -F <(echo 'int func(int, int, array(int, arg2));') ./ble
> ble->func(1, 2, [ 1, 2 ]) = 2
> +++ exited (status 2) +++
> $ ./ltrace -a0 -efunc -F <(echo 'int func(+int, int, array(int, arg2));') ./ble
> ble->func(1, 2, [ 1 ]) = 2
> +++ exited (status 2) +++
> 
> It's unclear whether it's actually a bug (what arg* should actually mean
> is not written anywhere), but I find it unintuitive.  In any case,
> currently arg* refers to actual arguments, and it should stay so for
> backward compatibility.  The premise that there is any correspondence
> between formal and actual arguments is wrong anyway: parameter packs can
> expand to any number of actual arguments, including zero.
> 
> So the right fix seems to be in the reader:
> 
> --- a/read_config_file.c
> +++ b/read_config_file.c
> @@ -1049,8 +1049,9 @@ process_line(char *buf) {
>  		}
>  
>  		int own;
> -		struct arg_type_info *type = parse_lens(&str, &extra_param,
> -							fun->num_params, &own);
> +		struct arg_type_info *type
> +			= parse_lens(&str, &extra_param,
> +				     fun->num_params - have_stop, &own);
>  		if (type == NULL) {
>  			report_error(filename, line_no,
>  				     "unknown argument type");
> 
> Now, this doesn't fully solve the issue either.  If there is another
> pack before "format", things will break again.  There are no packs other
> than format at the moment, and that is only allowed once, but in
> principle, nothing prevents me from creating a C function with an
> interface like this:
> 
>   void weird(char const *fmt1, ..., char const *fmt2, ...);
> 
> (That's illegal C, of course, but I could make a function that behaves
> this way.)  In ltrace, this would be:
> 
>   void weird(format, format);
> 
> Easy.  Except the latter format will look for its format string in arg2,
> because currently, format expands to something like:
> 
>   int printf(string, __format_pack(argN));
> 
> Where N is whatever the reader thinks is number of the string argument,
> which is of course number of _formal_ arguments seen so far.  When
> ltrace groks named arguments, it could instead become:
> 
>   int printf(tmp=string, __format_pack(tmp));
> 
> Which would handle "weird" above as well:
> 
>   void weird(tmp1=string, __format_pack(tmp1), tmp2=string, __format_pack(tmp2));
> 
> Supporting "format(...)" as pack syntax, keeping the behavior of plain,
> argument-less "format" for backward compatibility, would allow even
> weirder things like:
> 
>   void weirder(fmt1=string, fmt2=string, format(fmt1), format(fmt2));
> 
> So, that's about enough of pies in the sky for one day ;)
> 
> I pushed your test case, the abort-on-unsupported patch, and my fix.  I
> won't push the vector growth patch.  vect is supposed to be useful as a
> general growable array, and pushback should therefore have O(1)
> amortized complexity.


Great, thanks alot for looking at this!

Cheers
Edgar



More information about the Ltrace-devel mailing list