|
From: Nicholas N. <nj...@ca...> - 2002-10-29 09:12:11
|
Hi,
I've been thinking about the problem of ensuring skins and the core work
together across different versions, etc.
Here's my best idea so far. Have two `details' fields that must be filled
in by a skin, core_versionv and core_versionc, a bit like argv and argc,
that define a set of core versions that a skin can work with. Eg:
details->core_versionc = 2;
details->core_versionv = { "1.1.0", "1.1.1" };
The core version would always be a string of the form "X.Y.Z". The core
would bomb out if its version doesn't match any of the core_versionv
entries.
This allows a skin to work with more than one version of the core, which
seems like a good thing. Skins would have to be updated for new versions
of the core, even though this may not always be necessary, but this seems
ok; it's probably better to sometimes bomb out unnecessarily saying
"please get an updated version of the skin" than to not bomb out and then
get some mysterious crash.
Jeremy, Josef: how does this sound to you? Can you think of any flaws?
It would be nice to get this right on the first try.
N
|
|
From: Josef W. <Jos...@gm...> - 2002-10-29 13:17:10
|
On Tuesday 29 October 2002 10:12, Nicholas Nethercote wrote:
> Hi,
>
> I've been thinking about the problem of ensuring skins and the core wor=
k
> together across different versions, etc.
>
> Here's my best idea so far. Have two `details' fields that must be fil=
led
> in by a skin, core_versionv and core_versionc, a bit like argv and argc=
,
> that define a set of core versions that a skin can work with. Eg:
>
> details->core_versionc =3D 2;
> details->core_versionv =3D { "1.1.0", "1.1.1" };
>
> The core version would always be a string of the form "X.Y.Z". The cor=
e
> would bomb out if its version doesn't match any of the core_versionv
> entries.
>
> This allows a skin to work with more than one version of the core, whic=
h
> seems like a good thing. Skins would have to be updated for new versio=
ns
> of the core, even though this may not always be necessary, but this see=
ms
> ok; it's probably better to sometimes bomb out unnecessarily saying
> "please get an updated version of the skin" than to not bomb out and th=
en
> get some mysterious crash.
Another idea: What speaks against adopting the versioning scheme of share=
d=20
objects? A skin uses the core a little like a shared library. On compilin=
g=20
the skin, the core version is in skin.h (say X1.Y1.Z1). This is the minim=
um=20
version of the core required.
Another revision of the core X1.Y1.Z2 (Z1 !=3D Z2) is "binary compatible"=
(bugs=20
fixed).
A new minor version of the core X1.Y2.Z2 (Y2>Y1) keeps the same=20
callbacks/structures but adds a few core functions; thus it's "upwards bi=
nary=20
compatible" to a skin compiled with core version X1.Y1.Z1.
For every thing else the core gets a new major version and you need to ha=
ve a
new skin.
This has the advantage that the skin programmer doesn't have to specify a=
ny=20
core versions explicitly. He makes sure the skin runs with the core versi=
on=20
he had the skin compiled with. And that's enough (Same as "ld" inserts th=
e=20
shared object version numbers needed).
At valgrind installation, we make the same links for valgrind.so as when =
a=20
shared lib is installed: I.e. valgrind.so.1 -> valgrind.so.1.1. This way=20
different versions of valgrind can be installed simultaniously. Either us=
e a=20
dispatching "valgrind" script with a "--version=3D" option that loads the=
=20
correct valgrind.so. Or (IMHO better) each installed skin provides its=20
startup script and loads the "valgrind.so.<major>" it needs.
Perhaps I'm overlooking something.=20
But I think with a somewhat stable skin/core interface there will be more=
=20
skins written; nobody wants to adjust his skin every few months. At least=
, a=20
skin should work with new core revisions with same minor/major number.
Bad thing with this approach: when the core will change rapidly in the fu=
ture,=20
the major versions increase very fast.
> Jeremy, Josef: how does this sound to you? Can you think of any flaws=
?
What's the reason you want exact version dependency?
> It would be nice to get this right on the first try.
Definitly.
Josef
PS: Nick, do you remember the workaround I have in vg_symtab2.c for wrong=
=20
generated STABS debugging line info of GCC 2.95.3 (the line numbers don't=
=20
cover the prolog of a function).
I recently looked around in GDB for the DWARF2 reader. And I saw they hav=
e the=20
same workaround (in gdb/dwarf2read.c).
=09/* This function exists to work around a bug in certain compilers
=09 (particularly GCC 2.95), in which the first line number marker of a
=09 function does not show up until after the prologue, right before
=09 the second line number marker. This function shifts ADDRESS down
=09 to the beginning of the function if necessary, and is called on
=09 addresses passed to record_line. */
=09static CORE_ADDR
=09check_cu_functions (CORE_ADDR address)
=09...
Obviously GCC 2.95 can generate DWARF2 if requested? And then has the sam=
e=20
bug. Therefore, I would like to have this workaround in HEAD (for STABS).
Valgrind never had a problem with this, because in a backtrace, no addres=
ses=20
on the stack will ever point to function prologs. But I'm using the debug=
=20
info of the prolog address to construct the calltree :-(
What do you think on this issue?
I can provide the patch. Unfortunately I don't have GCC 2.95.X installed =
any=20
more. Will have to install again to do the testing (perhaps even provide =
a=20
patch for DWARF2 if GCC 2.95.3 can produce DWARF2 info...)
>
> N
>
>
>
> -------------------------------------------------------
> This sf.net email is sponsored by:ThinkGeek
> Welcome to geek heaven.
> http://thinkgeek.com/sf
> _______________________________________________
> Valgrind-developers mailing list
> Val...@li...
> https://lists.sourceforge.net/lists/listinfo/valgrind-developers
|
|
From: Nicholas N. <nj...@ca...> - 2002-10-29 13:37:40
|
On Tue, 29 Oct 2002, Josef Weidendorfer wrote: > Another idea: What speaks against adopting the versioning scheme of shared > objects? A skin uses the core a little like a shared library. On compiling > the skin, the core version is in skin.h (say X1.Y1.Z1). This is the minimum > version of the core required. > > Another revision of the core X1.Y1.Z2 (Z1 != Z2) is "binary compatible" (bugs > fixed). > A new minor version of the core X1.Y2.Z2 (Y2>Y1) keeps the same > callbacks/structures but adds a few core functions; thus it's "upwards binary > compatible" to a skin compiled with core version X1.Y1.Z1. > For every thing else the core gets a new major version and you need to have a > new skin. > > This has the advantage that the skin programmer doesn't have to specify any > core versions explicitly. He makes sure the skin runs with the core version > he had the skin compiled with. And that's enough (Same as "ld" inserts the > shared object version numbers needed). > > At valgrind installation, we make the same links for valgrind.so as when a > shared lib is installed: I.e. valgrind.so.1 -> valgrind.so.1.1. This way > different versions of valgrind can be installed simultaniously. Either use a > dispatching "valgrind" script with a "--version=" option that loads the > correct valgrind.so. Or (IMHO better) each installed skin provides its > startup script and loads the "valgrind.so.<major>" it needs. > > Perhaps I'm overlooking something. > But I think with a somewhat stable skin/core interface there will be more > skins written; nobody wants to adjust his skin every few months. At least, a > skin should work with new core revisions with same minor/major number. > Bad thing with this approach: when the core will change rapidly in the future, > the major versions increase very fast. There's a conflict here between ease-of-use for core developers, skin developers, and core/skin users; there's also the question of how general and complex should this system be? My idea is simple and somewhat selfish -- ie. it will make my life easy. Your idea is more general and powerful, but also more complex and makes my life more difficult. Since I am trying to be a PhD student first and a free software developer second, I am strongly inclined towards the simple idea. I don't particularly want to have to worry about binary compatibility. Also, Valgrind's current version numbers already have a rough meaning (X.Y.Z, Z means minor release, odd Y means development version, even Y means stable version, a la Linux kernel), so overloading the version numbers would be a pain. I don't mean to sound belligerent; I'm just trying to avoid digging holes that I will fall into later :) > > Jeremy, Josef: how does this sound to you? Can you think of any flaws? > > What's the reason you want exact version dependency? To ruthlessly minimise the possibility of Valgrind crashing due to core/skin version mismatches. Abortion-of-execution-with-explanation is not good, but mysterious crashes are much worse. > PS: Nick, do you remember the workaround I have in vg_symtab2.c for wrong > generated STABS debugging line info of GCC 2.95.3 (the line numbers don't > cover the prolog of a function). > I recently looked around in GDB for the DWARF2 reader. And I saw they have the > same workaround (in gdb/dwarf2read.c). > > /* This function exists to work around a bug in certain compilers > (particularly GCC 2.95), in which the first line number marker of a > function does not show up until after the prologue, right before > the second line number marker. This function shifts ADDRESS down > to the beginning of the function if necessary, and is called on > addresses passed to record_line. */ > > static CORE_ADDR > check_cu_functions (CORE_ADDR address) > ... > > Obviously GCC 2.95 can generate DWARF2 if requested? And then has the same > bug. Therefore, I would like to have this workaround in HEAD (for STABS). > Valgrind never had a problem with this, because in a backtrace, no addresses > on the stack will ever point to function prologs. But I'm using the debug > info of the prolog address to construct the calltree :-( > What do you think on this issue? I'm confused... is the bug for DWARF2 or stabs? Is your patch for DWARF2 or stabs? If Valgrind has a bug/imperfection and you have a patch, then I guess using it would be good... > I can provide the patch. Unfortunately I don't have GCC 2.95.X installed any > more. Will have to install again to do the testing (perhaps even provide a > patch for DWARF2 if GCC 2.95.3 can produce DWARF2 info...) N |
|
From: Josef W. <Jos...@gm...> - 2002-10-29 15:18:09
|
On Tuesday 29 October 2002 14:37, Nicholas Nethercote wrote: > ... > There's a conflict here between ease-of-use for core developers, skin > developers, and core/skin users; there's also the question of how gene= ral > and complex should this system be? Yes: =46rom a users view, the shared object versioning is the best: He can ins= tall a=20 few skins, no matter if they require different valgrind core major versio= ns,=20 as they could be installed simultaniously. =46rom a skin developers view, getting the "minimum required version" fro= m=20 skin.h and keeping the skin/core interface stable among minor releases, i= s=20 the best. > My idea is simple and somewhat selfish -- ie. it will make my life easy= =2E > Your idea is more general and powerful, but also more complex and makes= my But I hope you won't change the core/skin interface in minor versions? Otherwise it will be a nightmare for skin developers. At least the minor version check thus should be automatic. And let the sk= in=20 spezify only X.Y versions of the core it is compatible with. Or simpler: = Only=20 let the skin work with the X.Y version it was compiled with. > ... > To ruthlessly minimise the possibility of Valgrind crashing due to > core/skin version mismatches. Abortion-of-execution-with-explanation i= s > not good, but mysterious crashes are much worse. You could give out a warning if the minor versions don't match. > ... > I'm confused... is the bug for DWARF2 or stabs? Is your patch for DWAR= F2 > or stabs? GCC 2.95.x obviously generates bad line info for prologues, no matter if = STABS=20 or DWARF2 format is used. As STABS is the default for GCC 2.95.x, I only = used=20 this one in my GCC 2.95.3 days, and thus only have a patch for the STABS=20 reader. Josef |
|
From: Jeremy F. <je...@go...> - 2002-10-29 16:08:12
|
On Tue, 2002-10-29 at 01:12, Nicholas Nethercote wrote:
Hi,
I've been thinking about the problem of ensuring skins and the core work
together across different versions, etc.
Here's my best idea so far. Have two `details' fields that must be filled
in by a skin, core_versionv and core_versionc, a bit like argv and argc,
that define a set of core versions that a skin can work with. Eg:
details->core_versionc = 2;
details->core_versionv = { "1.1.0", "1.1.1" };
[ Strictly syntactic comment: this should be a NULL terminated array
rather than counted; manually maintaining the correspondence is one of
those rich sources of minor irritating bugs ]
The core version would always be a string of the form "X.Y.Z". The core
would bomb out if its version doesn't match any of the core_versionv
entries.
This allows a skin to work with more than one version of the core, which
seems like a good thing. Skins would have to be updated for new versions
of the core, even though this may not always be necessary, but this seems
ok; it's probably better to sometimes bomb out unnecessarily saying
"please get an updated version of the skin" than to not bomb out and then
get some mysterious crash.
This seems fragile and hard to maintain. The problem is that the skin
developer's tendency will to simply add the new version number onto the
end of the list without really checking to see if all the old versions
still work; this will be exacerbated if you require an exact match
between core and skin versions. Also, it will make stable skins rot
very quickly if a minor update to core requires changes to all skins.
I think a better scheme would be to have the core export a preprocessor
define called something like VG_IF_VERSION. The skins would then simply
do:
details->core_interface = VG_IF_VERSION;
If the core makes an incompatible change to its skin interface, then
increment the IF_VERSION. You could use a string in X.Y format, where
changes in X are completely incompatible, but Y indicates a backwards
compatible change.
This way the association between skin binary and core is checked by the
compiler and then built into the skin automatically. So long as the
interface doesn't expose any structures which can change shape, this
should work well.
If you want to get fancy and have the core expose multiple incompatible
interface versions, you'd need to do something like:
/* My skin */
#define VG_WANT_IF 2
#include "vg_skin.h"
//...
details->core_interface = VG_IF_VERSION; /* major = 2 */
But I don't think that's terribly necessary.
J
|
|
From: Nicholas N. <nj...@ca...> - 2002-10-29 16:23:33
|
On 29 Oct 2002, Jeremy Fitzhardinge wrote: > I think a better scheme would be to have the core export a preprocessor > define called something like VG_IF_VERSION. The skins would then simply > do: > > details->core_interface = VG_IF_VERSION; > > If the core makes an incompatible change to its skin interface, then > increment the IF_VERSION. You could use a string in X.Y format, where > changes in X are completely incompatible, but Y indicates a backwards > compatible change. > > This way the association between skin binary and core is checked by the > compiler and then built into the skin automatically. Hmm, I like this. Decoupling the interface version and the Valgrind version is a v. good idea, as is the X vs. Y version changes compared to the set of compatible versions. It also means the skin writer doesn't need to worry about tracking core versions because it's done automatically. > So long as the interface doesn't expose any structures which can change > shape, this should work well. Not sure if I understand this fully... only problem I can think of is that the `details' struct changed so that the `core_interface' field was in a different place. The `details' struct is the first argument to VG_(pre_clo_init) so changing the other structs shouldn't matter, I think. And if I make `core_interface' the first field in `details' that should even work if more fields are added to the end of `details' later on? > If you want to get fancy nope :) ---- BTW, my understanding of what constitutes a backward (in)compatible change is hazy with shared objects... here's what I'm guessing is backward compatible: add a local function to core: yes add a global function to core: yes change a global function's args in core: no change a data structure (eg. struct) seen by skins: no There are sure to be other things I haven't thought of here. It seems like there are two levels of incompatibility, too: a. changes which require a skin to be recompiled b. changes which require a skin's code to be changed An example of (a) would be adding a new (optional) field to the `needs' struct. An example of (a) would be adding a new (compulsory) field to the `details' struct. Am I on the right track? Any other comments, etc? Thanks. N |
|
From: Jeremy F. <je...@go...> - 2002-10-29 17:10:25
|
On Tue, 2002-10-29 at 08:23, Nicholas Nethercote wrote:
Hmm, I like this. Decoupling the interface version and the Valgrind
version is a v. good idea, as is the X vs. Y version changes compared to
the set of compatible versions. It also means the skin writer doesn't
need to worry about tracking core versions because it's done
automatically.
Yep.
> So long as the interface doesn't expose any structures which can change
> shape, this should work well.
Not sure if I understand this fully... only problem I can think of is that
the `details' struct changed so that the `core_interface' field was in a
different place. The `details' struct is the first argument to
VG_(pre_clo_init) so changing the other structs shouldn't matter, I think.
And if I make `core_interface' the first field in `details' that should
even work if more fields are added to the end of `details' later on?
Well, I was thinking more of how to make sure that the compiler will
pick up as many incompatible changes for itself. An example of a
troublesome interface would be where the skin creates an instance of a
structure and passes it back to the core. Also, a function which
changes the meaning of its arguments without changing their type would
cause a silent incompatibility (in that the compiler wouldn't notice).
But you're right; so long as the version discriminator field is first,
it doesn't much matter what goes on elsewhere (if you're willing to
break interface compatibility).
BTW, my understanding of what constitutes a backward (in)compatible change
is hazy with shared objects... here's what I'm guessing is backward
compatible:
add a local function to core: yes
If by "local function" you mean something which isn't part of the skin
interface, then yes.
add a global function to core: yes
change a global function's args in core: no
change a data structure (eg. struct) seen by skins: no
Maybe. If the struct is allocated by the core, only has new elements
added to the end, and they're initialized to sane default values, then
that's compatible. details and needs are examples of such structures
(so long as good discipline in making sure that new elements always go
at the end). You'd have to code it something like:
struct thing_v1 {
/* 1.0 functions */
int (*foo)(void);
/* 1.1 functions */
int (*bar)(int, int, char *);
void (*clunk)(char *, void *, int ****[], void (*foo)(int));
/* 1.2 functions */
int (*__obsolete_1)(int, int, void *);
};
Also, enums: always add new elements at the end. In fact, its better to
make sure that enums are always explicitly enumerated to get good
interface stability: enum { foo=0, bar=1, biff=4, bunk=3 }
There are sure to be other things I haven't thought of here.
It seems like there are two levels of incompatibility, too:
a. changes which require a skin to be recompiled
b. changes which require a skin's code to be changed
An example of (a) would be adding a new (optional) field to the `needs'
struct.
This doesn't need a recompile if the order of existing fields is
unaffected.
An example of (a) would be adding a new (compulsory) field to the
`details' struct.
Am I on the right track? Any other comments, etc? Thanks.
Yes. Not exposing structures as part of the interface is a really good
idea (OpenGL is a really nice example of an API which is extremely good
at maintaining long-term binary compatibility, partly because it exposes
no structures more complex than arrays of ints or floats).
J
|
|
From: Nicholas N. <nj...@ca...> - 2002-10-30 10:31:57
|
On some days, Jeremy Fitzhardinge wrote:
> I think a better scheme would be to have the core export a preprocessor
> define called something like VG_IF_VERSION. The skins would then simply
> do:
>
> details->core_interface = VG_IF_VERSION;
>
> If the core makes an incompatible change to its skin interface, then
> increment the IF_VERSION. You could use a string in X.Y format, where
> changes in X are completely incompatible, but Y indicates a backwards
> compatible change.
>
> This way the association between skin binary and core is checked by the
> compiler and then built into the skin automatically.
Thinking about this some more, seems to me like every skin is going to
identically do:
details->core_interface = VG_IF_VERSION;
In which case why not just put:
const Char* core_interface = VG_IF_VERSION;
in vg_skin.h, which every skin #includes. That way, the skin doesn't have
to mention anything about version numbers, and assuming we get the X.Y
binary (in)compatibility numbering right, everything will work as desired.
(Thinking some more: it would be better to have VG_MINOR_IF_VERSION and
VG_MAJOR_IF_VERSION integers rather than a string, just to make
comparisons simpler.)
> Not exposing structures as part of the interface is a really good
> idea (OpenGL is a really nice example of an API which is extremely good
> at maintaining long-term binary compatibility, partly because it exposes
> no structures more complex than arrays of ints or floats).
Interesting. Maybe I should change the way `details', `needs' and
`trackable events' are set, using functions instead? Eg. change this:
details->name = "foo";
details->version = "1.1.1";
details->description = "blah blah blah";
details->copyright_author =
"Copyright (C) 2002, and GNU GPL'd, by Nicholas Nethercote.";
details->bug_reports_to = "nj...@ca...";
needs->basic_block_discards = True;
needs->command_line_options = True;
track->new_mem_heap = & my_new_mem_heap;
track->die_mem_heap = & my_die_mem_heap;
to this:
VG_(details_name)("foo");
VG_(details_version)("1.1.1");
...
VG_(set_needs_basic_block_discards)();
VG_(set_command_line_options)();
/* could have corresponding VG_(unset_command_line_options)() etc */
VG_(track_new_mem_heap)(& my_new_mem_heap);
VG_(track_die_mem_heap)(& my_die_mem_heap);
And not expose the fields of the struct in vg_skin.h. That way even if the
structs were totally reordered in core, an old skin would still work fine
with it.
The only complication with this is that a skin could call these functions
at any time... but that could possibly be desirable (eg. start/stop
tracking new_mem_heap events if something happens) and it's also currently
possible anyway since VG_(needs) etc. are all global anyway. By using
functions I could add some sanity checking, eg. complain if a skin tries
to set a need after startup which would screw things up (eg. shadow_regs,
which affects the baseBlock initialisation and thus can't be turned on
halfway through execution).
There are some other exposed structs in the interface... I might have a
look through and see how else it can be better future-proofed.
Thanks for the good suggestions.
N
|
|
From: Jeremy F. <je...@go...> - 2002-10-30 16:07:06
|
On Wed, 2002-10-30 at 02:31, Nicholas Nethercote wrote:
Thinking about this some more, seems to me like every skin is going to
identically do:
details->core_interface = VG_IF_VERSION;
In which case why not just put:
const Char* core_interface = VG_IF_VERSION;
[ Well, SK_(core_interface) ]. Yes, that would work.
in vg_skin.h, which every skin #includes. That way, the skin doesn't have
to mention anything about version numbers, and assuming we get the X.Y
binary (in)compatibility numbering right, everything will work as desired.
(Thinking some more: it would be better to have VG_MINOR_IF_VERSION and
VG_MAJOR_IF_VERSION integers rather than a string, just to make
comparisons simpler.)
Yes, then you can do CPP checking:
#if VG_MAJOR_IF_VERSION < 3
#error Need Valgrind version 3
#endif
And not expose the fields of the struct in vg_skin.h. That way even if the
structs were totally reordered in core, an old skin would still work fine
with it.
Yes, that would be better. A little more cumbersome, but it could be
prettied up with a macro perhaps.
J
|
|
From: Nicholas N. <nj...@ca...> - 2002-11-05 11:36:23
|
Hi,
Here's a patch implementing the discussed core/skin interface versioning
system. I'm putting it here because I would appreciate some sanity
checking before I commit it; I find this sort of thing easy to get wrong.
The only changes are to VG_(main)() (at the very start) and to
include/vg_skin.h. Existing skins that don't define the new VG_(skin...)
symbols work ok with an updated core (don't ask me why); when both the
skin and the core have been updated it complains if their major version
numbers don't match.
Jeremy, you mentioned using CPP checking, a la:
#if VG_MAJOR_IF_VERSION < 3
#error Need Valgrind version 3
#endif
But I don't quite see how to fit this in; my approach is a little
different. Perhaps I am missing something obvious and it can be done more
simply.
N
Index: coregrind/vg_main.c
===================================================================
RCS file: /cvsroot/valgrind/valgrind/coregrind/vg_main.c,v
retrieving revision 1.62
diff -C2 -r1.62 vg_main.c
*** coregrind/vg_main.c 3 Nov 2002 11:44:36 -0000 1.62
--- coregrind/vg_main.c 5 Nov 2002 11:26:39 -0000
***************
*** 1319,1322 ****
--- 1319,1342 ----
ThreadState* tst;
+ /* Check skin and core versions are compatible */
+ if (VG_CORE_INTERFACE_MAJOR_VERSION != VG_(skin_interface_major_version)) {
+ VG_(printf)("Error:\n"
+ " Skin and core interface versions do not match.\n"
+ " Interface version used by core is: %d.%d\n"
+ " Interface version used by skin is: %d.%d\n"
+ " The major version numbers must match.\n",
+ VG_CORE_INTERFACE_MAJOR_VERSION,
+ VG_CORE_INTERFACE_MINOR_VERSION,
+ VG_(skin_interface_major_version),
+ VG_(skin_interface_minor_version));
+ VG_(printf)(" You need to at least recompile, and possibly update,\n");
+ if (VG_CORE_INTERFACE_MAJOR_VERSION > VG_(skin_interface_major_version))
+ VG_(printf)(" your skin to work with this version of Valgrind.\n");
+ else
+ VG_(printf)(" your version of Valgrind to work with this skin.\n");
+ VG_(printf)(" Aborting, sorry.\n");
+ VG_(exit)(1);
+ }
+
/* Set up our stack sanity-check words. */
for (i = 0; i < 10; i++) {
Index: include/vg_skin.h
===================================================================
RCS file: /cvsroot/valgrind/valgrind/include/vg_skin.h,v
retrieving revision 1.27
diff -C2 -r1.27 vg_skin.h
*** include/vg_skin.h 27 Oct 2002 20:28:29 -0000 1.27
--- include/vg_skin.h 5 Nov 2002 11:26:39 -0000
***************
*** 107,110 ****
--- 107,135 ----
/*====================================================================*/
+ /*=== Core/skin interface version ===*/
+ /*====================================================================*/
+
+ /* The major version number indicates binary-incompatible changes to the
+ interface; if the core and skin major versions don't match, Valgrind
+ will abort. The minor version indicates binary-compatible changes.
+
+ We don't want the variables themselves in the core, only in the skins,
+ hence the #ifndef. But the core needs to know of their existence, hence
+ the #else branch. Phew.
+ */
+
+ #define VG_CORE_INTERFACE_MAJOR_VERSION 1
+ #define VG_CORE_INTERFACE_MINOR_VERSION 1
+
+ #ifndef __VG_INCLUDE_H
+ const Int VG_(skin_interface_major_version) = VG_CORE_INTERFACE_MAJOR_VERSION;
+ const Int VG_(skin_interface_minor_version) = VG_CORE_INTERFACE_MINOR_VERSION;
+ #else
+ extern const Int VG_(skin_interface_major_version);
+ extern const Int VG_(skin_interface_minor_version);
+ #endif /* __VG_INCLUDE_H */
+
+
+ /*====================================================================*/
/*=== Command-line options ===*/
/*====================================================================*/
|
|
From: Jeremy F. <je...@go...> - 2002-11-05 17:32:49
|
On Tue, 2002-11-05 at 03:36, Nicholas Nethercote wrote:
Hi,
Here's a patch implementing the discussed core/skin interface versioning
system. I'm putting it here because I would appreciate some sanity
checking before I commit it; I find this sort of thing easy to get wrong.
The only changes are to VG_(main)() (at the very start) and to
include/vg_skin.h. Existing skins that don't define the new VG_(skin...)
symbols work ok with an updated core (don't ask me why); when both the
skin and the core have been updated it complains if their major version
numbers don't match.
So as I understand it, the idea is that including vg_skin.h will
magically define you a version variable?
Two comments:
- I think it should be called SK_(skin_interface_major_version), just so
that it is obvious who's doing the defining.
- It seems to me that this won't work except for skins with a single
source file. If a skin has multiple sources all of which include
vg_skin.h, won't it end up with multiple definitions of
skin_interface_major_version?
Jeremy, you mentioned using CPP checking, a la:
#if VG_MAJOR_IF_VERSION < 3
#error Need Valgrind version 3
#endif
But I don't quite see how to fit this in; my approach is a little
different. Perhaps I am missing something obvious and it can be done more
simply.
Well, it would be nice to have something a little more explicit that a
bunch of errors about prototype mismatches and missing functions.
J
|
|
From: Nicholas N. <nj...@ca...> - 2002-11-06 10:12:30
|
On 5 Nov 2002, Jeremy Fitzhardinge wrote: > - It seems to me that this won't work except for skins with a single > source file. If a skin has multiple sources all of which include > vg_skin.h, won't it end up with multiple definitions of > skin_interface_major_version? I don't think so, because vg_skin.h is enclosed inside: #ifndef __VG_SKIN_H #define __VG_SKIN_H ... #endif > #if VG_MAJOR_IF_VERSION < 3 > #error Need Valgrind version 3 > #endif > > But I don't quite see how to fit this in; my approach is a little > different. Perhaps I am missing something obvious and it can be done more > simply. > > Well, it would be nice to have something a little more explicit that a > bunch of errors about prototype mismatches and missing functions. I didn't make myself clear; I don't see where such code should go... would it be in vg_skin.h? Somewhere else? Also, the way I've done it, the errors occur at run-time, and you get an error message like this at startup: Error: Skin and core interface versions do not match. Interface version used by core is: 2.1 Interface version used by skin is: 1.1 The major version numbers must match. You need to at least recompile, and possibly update, your skin to work with this version of Valgrind. Aborting, sorry. N |