[rrd-developers] [PATCH] Suggestion for API extension (rrd_dump)

[Benny Baumann]

Hi,

Am 17.08.2009 00:21, schrieb Tobias Oetiker:
> Hi Benny,
>
> I finally got the time to look at your patch, the call back
> interface as such seems ok to me ... (I have never written one
> myself, so I don;t know if we should look out for certain gotchas
> ... )
>
> the implementation in rrd_dump does not strike me as all that
> elegant ...
>   
I know it's not that elegant right now, especially because of the string
literals ...
> repeating the same string .... hmmm
>
> +        cb("<!DOCTYPE rrd SYSTEM \"http://oss.oetiker.ch/rrdtool/rrdtool.dtd\">\n",
> +            strlen("<!DOCTYPE rrd SYSTEM \"http://oss.oetiker.ch/rrdtool/rrdtool.dtd\">\n"), user);
>
>
> also the combination
>
> snprintf(somestring_buf, 255, ...)
> cb(somestring_buf, strlen(somestring_buf), ...)
>
> apears about 1000 times ...
> how about making a function or at least a #define to make this look
> a bit less repetitive ...
>   
I thought so too, but didn't do this for now to minize the changes in
the patch to as trivial ones as possible. Adding 2 #defines to cancel
out the repeatition should be the next step IMHO.

#define cb_strlit(str) cb(str, strlen(str), user)
#define cb_strbuf() cb(somestring_buf, strlen(somestring_buf), user)

Those macros should do (as local shortcuts). Pushing the snprintf into
the cb_strbuf macro isn't a good idea, because snprintf is a VarArgs
function, thus unnecessarily complicating the macro if we would pass
things through.
> also I think the new function should go into librrd.sym.in.in
>   
I wasn't sure where this should be registered as I don't programm Linux
libraries that often. Just add it there.

But besides the rrd_dump_cb_r; what about adding the rrd_dump_opt_r
function too? I mainly ask because when writing an API wrapper you often
already have preprocessed information about what you want the final
function call to do thus you can skip some unnecessary conversion steps
in between.
> cheers
> tobi
>   
Regards,
BenBE.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
Url : http://lists.oetiker.ch/pipermail/rrd-developers/attachments/20090817/37eeec0d/attachment.pgp