|
From: Julian S. <js...@ac...> - 2005-03-28 18:41:05
|
Replying to several messages at once, up to and including discussion
of the proposed patch.
Simplifying the tool interface is definitely a Good Thing To Do. However
I'm not convinced by the objections raised thus far to the struct passing
approach, and I like its simplicity. Reasons below.
> > The "register" approach means (if I understand correctly) that
> > the tool's init function is called, resulting in a bunch of calls
> > back to the core to register functions; eventually the tool init
> > fn returns. That sounds more complex (mutable state; more complex
> > control flow) and I'm not sure what the benefit is.
>
> One problem with the dictionary approach is that you are exposing the
> internals of the struct to tools. I avoided this approach with the
> 'needs' and 'details' structs for just this reason.
Confused. Each tool would assemble a struct and pass a pointer
to it to the core as the return value of the tool's initialisation
function. So inevitably the internals of the struct are visible
to the tool anyway since it created the struct.
> Yes, I think that's better because it removes the fragility of having a
> structure as part of the interface.
How does having a struct as part of the interface make it fragile?
I am thinking of a very simple struct which contains only function
pointers; no other data.
If you are really worried about version mismatches in the struct,
make the first field (viz, offset 0) be an int which carries a
version identifier. Then we can safely say
"if (struct->version != ...) barf()"
This at least allows detection of ABI-level mismatches at startup
time, providing the version number is bumped each time the API changes.
> And this isn't any more robust than passing a structure against adding
> new interface functions (it's worse, because adding a new function would
> require every caller to VG_(needs_tool_errors)() be changed, even if
> they don't care about the new function; a static struct will
> automatically initialize otherwise uninitialized members to NULL).
I agree. Especially if the struct has a version field at the start.
------
The registration-based scheme feels cluttered to me, because there are
more names and storage locations (global state) to keep track of. For
example, the tool makes a registration call:
+ VG_(basic_tool_funcs) (TL_(post_clo_init),
+ TL_(instrument),
+ TL_(fini));
Now in the core, if I want to call one of these, first I have to find the
global variables that those pointers got stored in -- probably using
grep. Then I have to be convinced that they've been initialised, and
only then can I make the call. Also I have to figure out precisely
which groups of functions make up the interface, and whether I've
forgotten to consider any.
By comparison, if I have a struct "tdict" from the tool, then there are
no banks of global variables to be found, no questions about initialisation,
and if I want assurance I'm seeing the complete interface, I only have to
read the struct definition -- nothing else. Then I can do:
assert(tdict->instrument); // we're hosed if this fails
bbi = tdict->instrument(bb);
or
if (tdict->some_optional_function)
some_optional_function(args);
------
Re-reading that, my real point is that the struct scheme is easier for
human readers of the code to read and reason about. And as Jeremy says,
it's not really any less robust against API changes relative to the
registration-scheme.
J
|