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

[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 ...

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 ...

also I think the new function should go into librrd.sym.in.in


cheers
tobi

> Short correction since I missed some changes in the original patch:
>
> Regards,
> BenBE.
>
> Index: a/src/rrd_dump.c
> ===================================================================
> --- a/src/rrd_dump.c    (old)
> +++ b/src/rrd_dump.c    (new)
> @@ -120,12 +120,6 @@
>              strlen("<rrd>\n"), user);
>      }
>
> -    cb("<!-- Round Robin Database Dump -->",
> -        strlen("<!-- Round Robin Database Dump -->"), user);
> -
> -    cb("<rrd>",
> -        strlen("<rrd>"), user);
> -
>      if (atoi(rrd.stat_head->version) <= 3) {
>          snprintf(somestring, 255, "\t<version>%s</version>\n",
> RRD_VERSION3);
>      } else {
> @@ -207,8 +207,8 @@
>              strlen("\t</ds>\n\n"), user);
>      }
>
> -    cb("<!-- Round Robin Archives -->\n",
> -        strlen("<!-- Round Robin Archives -->\n"), user);
> +    cb("\t<!-- Round Robin Archives -->\n",
> +        strlen("\t<!-- Round Robin Archives -->\n"), user);
>
>      rra_base = rrd_file->header_len;
>      rra_next = rra_base;
>
>
>

-- 
Tobi Oetiker, OETIKER+PARTNER AG, Aarweg 15 CH-4600 Olten, Switzerland
http://it.oetiker.ch tobi at oetiker.ch ++41 62 775 9902 / sb: -9900