[rrd-developers] it exposes too much ...

[Florian Forster]

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/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rrd_test.h
Type: text/x-chdr
Size: 1057 bytes
Desc: not available
Url : http://lists.oetiker.ch/pipermail/rrd-developers/attachments/20080610/ee1dec19/attachment.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://lists.oetiker.ch/pipermail/rrd-developers/attachments/20080610/ee1dec19/attachment-0001.bin