[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