Hi again ;) On Tue, Jun 10, 2008 at 02:25:53PM +0200, Tobias Oetiker wrote: > they will not be removed just like that, they will be replaced by > 'good functions' with the same intention as the current non supported > / non documented functions. In other words, not exporting the > functions now, would cause the users to have no way but duplicating > rrdtool code somehow or compiling an alternete version ... in 1.4 they > will have the option to switch to the supported functions ...
My argument is all about giving the user the choice.. Leaving him/her with copying the source is a choice of some sort, but not an elegant one in my opinion. If the functions are replace with functions of the same or very similar functionality it's even better to declare those functions in the public interface. Then users (and by that I mean other programmers ;) can do: #if RRD_VERSION < 0x010400 /* Versions before 1.4.0 */ status = rrd_deprecated_function_13 (...); #else /* RRD_VERSION >= 0x010400 */ status = rrd_function_14 (...); #endif This will make adaption much easier. The alternative, using exported but not declared functions, is: - Check librrd version in the configure script - Conditionally enable building of the shipped copy of librrd - #if SHIPPED_LIBRRD # include "shipped/librrd-1.3/rrd.h" # include "shipped/librrd-1.3/rrd_format.h" #else # include <rrd-1.4/rrd.h> #endif - Conditionally link against librrd-1.3/librrd.a This is obviously cumbersome. So what people will do is: #include <rrd.h> extern int rrd_hidden_function (...); In the first alternative, the shared object isn't used at all. It can't really be used because no one knows if the system wide version behaves as the shipped headers suggest. So entirely removing the symbols from the shared object will not break this. The second version is just scary. Maybe it's best if I post a specific example. I've written a small example header file that demonstrates how I'd do it, it's attached to this mail: Users that want to use `rrd_deprecated_function' need to define `RRD_EXPORT_DEPRECATED' before including the header file. Another possibility would be to move all those files into <rrd_deprecated.h> and include #warning "You code will be broken soon" in there. I like having all in one file better though. > that __attribute__((deprecated)) thing looks cool. How does it work ? > can you send a patch ? As far as I know __attribute__ is a GNU extension, but other compilers may have very similar functionality. The `deprecated' attribute lets GCC throw a warning if the function is used anywhere, informing the user at compile time that he's using code he shouldn't use. The double quotes have a purpose of being able to easily remove the piece with a macro. I'm using `RRD_ATTRIBUTE(x)' that's defined to `/**/' if the source is not compiled with GCC in my example. You can find more information about the __attribute__ syntax here: <http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html> If I get a list of deprecated functions I'd gladly do a patch for you. > There is another issue with documenting the 'exported / non-supported' > functions, they depend on rrd-format.h which means that rrd-format.h > would have to become an exported header which is realy not what I > intend ... I'd much rather include that bit in the `deprecated' part of a header file than exporting a function that returns a pointer to a unspecified structure.. As noted above: The ``shipped'' version of the header may or may not be right and you may and up with a catastrophe. > I think I understand your intention. You want things (however they > are) in the open. Yes and no: I want _all exported symbols_ in the open. My second suggestion was to remove the ``problematic'' functions and achieve a fully documented interface this way.. Regards, -octo -- Florian octo Forster Hacker in training GnuPG: 0x91523C3D http://verplant.org/
/* $Copyright, $License */ #ifndef RRD_TEST_H #define RRD_TEST_H #if __GNUC__ # define RRD_ATTRIBUTE(x) __attribute__(x) #else # define RRD_ATTRIBUTE(x) /**/ #endif int rrd_public_function (void); /* more ``stable'' functions */ /* * The following functions are _internal_ functions needed to read the raw RRD * files. Since they are _internal_ they may change with the file format and * will be replaced with a more general interface in RRDTool 1.4. Don't use * these functions unless you have good reasons to do so. If you do use these * functions you will have to adapt your code for RRDTool 1.4! * * To enable the deprecated functions define `RRD_EXPORT_DEPRECATED' before * including <rrd_test.h>. You have been warned! If you come back to the * RRDTool mailing list and whine about your broken application, you will get * hit with something smelly! */ #if RRD_EXPORT_DEPRECATED int rrd_deprecated_function (void) RRD_ATTRIBUTE((deprecated)); /* more deprecated functions */ #endif /* RRD_EXPORT_DEPRECATED */ #endif /* RRD_TEST_H */
signature.asc
Description: Digital signature
_______________________________________________ rrd-developers mailing list [email protected] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers
