Eric Blake writes:
> On 04/16/2013 07:51 AM, Lluís Vilanova wrote:
>> Add commandline options to control initial loading of dynamic instrumentation
>> library.
>>
>> Signed-off-by: Lluís Vilanova <vilan...@ac.upc.edu>
>> ---
>> @@ -688,6 +690,15 @@ static void usage(void)
>> #endif
>> "-bsd type select emulated BSD type FreeBSD/NetBSD/OpenBSD
>> (default)\n"
>> "\n"
>> +#if defined(CONFIG_TRACE_INSTRUMENT)
>> + "Tracing options:\n"
>> +#if defined(CONFIG_TRACE_INSTRUMENT_DYNAMIC)
>> + "-instr path load a dynamic trace instrumentation
>> library\n"
>> +#endif
>> + "-instr-arg string\n"
>> + " argument to dynamic trace instrumentation
>> library (can be given multiple times)\n"
>> + "\n"
> Why do you document -instr-arg unconditionally, but -instr only when
> dynamic trace support is enabled; especially since the text of
> -instr-arg mentions that it is tied to dynamic tracing?
>> @@ -852,6 +866,14 @@ int main(int argc, char **argv)
>> singlestep = 1;
>> } else if (!strcmp(r, "strace")) {
>> do_strace = 1;
>> +#if defined(CONFIG_TRACE_INSTRUMENT_DYNAMIC)
>> + } else if (!strcmp(r, "instr")) {
>> + instrument_path = argv[optind++];
>> +#endif
>> +#if defined(CONFIG_TRACE_INSTRUMENT)
>> + } else if (!strcmp(r, "instr-arg")) {
>> + instr_parse_args(argv[optind++], &instrument_argc,
>> &instrument_argv);
>> +#endif
> At least your parsing code matches your documentation, but it still
> feels weird.
You cannot load a library in non-dynamic mode, but you can still pass it some
arguments if it's in static mode (CONFIG_TRACE_INSTRUMENT accounts for all
cases), thus the "weirdness" :)
>> @@ -3378,6 +3397,14 @@ static const struct qemu_argument arg_table[] = {
>> {"R", "QEMU_RESERVED_VA", true, handle_arg_reserved_va,
>> "size", "reserve 'size' bytes for guest virtual address space"},
>> #endif
>> +#if defined(CONFIG_TRACE_INSTRUMENT_DYNAMIC)
>> + {"instr", "QEMU_INSTR", true, handle_arg_instrument,
>> + "path", "load a dynamic trace instrumentation library"},
>> +#endif
>> +#if defined(CONFIG_TRACE_INSTRUMENT)
>> + {"instr-arg", "QEMU_INSTR_ARGS", true, handle_arg_instrument_arg,
>> + "string", "argument to dynamic trace instrumentation library (can
>> be given multiple times)"},
> Another case of mentioning the term dynamic even when dynamic
> instrumentation is not enabled.
Fixed.
Thanks a lot,
Lluis
--
"And it's much the same thing with knowledge, for whenever you learn
something new, the whole world becomes that much richer."
-- The Princess of Pure Reason, as told by Norton Juster in The Phantom
Tollbooth
