|
From: Josef W. <Jos...@gm...> - 2003-07-28 08:06:40
|
On Saturday 26 July 2003 13:15, Nicholas Nethercote wrote:
> On Fri, 25 Jul 2003, Josef Weidendorfer wrote:
> > I think that "enums" used in the interface have to be handled carefully
> > if not totally avoided and replaced by functions.
>
> Yes, good point.
>
> > Binary compatible changes can be done by adding new enum values at the
> > end. Hmmm...
> > The last element of Opcode is "DUMMY_FINAL_UOPCODE". And the comment says
> > that skins can use this value to extend UCode. This means that *every*
> > addition of a new Opcode will result in a binary incompatible change of
> > the skin interface!
> > Better provide a function with an implementation like:
> > void vg_getFinalUOpcode() { return CCALL+1; }
> > where CCALL is replaced by the last core Opcode in a given Valgrind
> > release and remove "DUMMY_FINAL_UOPCODE" alltogether.
> > This will lead to a more stable interface.
>
> I don't quite understand how this would work. An enum must be a
> compile-time constant, it can't be set with a function. Or do you propose
> getting rid of the enum for extended UInstrs? Eg, for Memcheck, replace
> MemCheckOpcode (in mc_include.h) with a function? But then you couldn't
> use switch statements on the extended UInstrs, which would be a pain.
You are right.
By using a function for the first OpCode available for a skin, extented
opcodes are no longer compile-time constant. But by subtracting a
runtime constant (vg_getFinalUOpcode()), they get constant again.
So the skin would define its own enum for extended opcodes.
Opcode values first are checked for >= vg_getFinalUOpcode(), and
use 2 switch statements, adjusting the Opcode value for extented ones first
by subtracting vg_getFinalUOpcode().
> Another possibility would be to leave a gap, ie. make
> DUMMY_FINAL_UOPCODE=100 (or more) which would leave space for about 40 (or
Yes. Perhaps this is easier to do?
> more) new UInstrs. (And do likewise for other enums.) Problem with that
> is that some code relies on there being no gap between UInstrs and
> extended UInstrs -- e.g. VG_(print_UInstr_histogram)() is one, although
> it's not very important. There may be others.
I'm sure this can be handled. Before "DUMMY_FINAL_UOPCODE = 100", put
e.g. a LAST_CORE_OPCODE. This should allow to handle the gap in an easy
way?
> Josef, maybe you could help me understand your suggestion with some
> snippets of code, or even a patch? Thanks.
You did understand the problem and the suggestion ;-)
I just noted the compatibility problem with changing this enum, and I wanted
you to know.
Thanks for taking this serious.
Josef
>
> N
|