[rrd-developers] [PATCH 2/5] src/rrd_daemon.c: Add the "FETCH" command.

[kevin brintnall]

On Sat, May 23, 2009 at 01:04:25PM +0200, Florian Forster wrote:
> From: Florian Forster <octo at leeloo.lan.home.verplant.org>
> 
> The `FETCH' command can be used to execute a `rrd_fetch_r' on the server
> and will return the data via the network connection.

> +static int handle_request_fetch (listen_socket_t *sock, /* {{{ */
> +    char *buffer, size_t buffer_size)

For all handler functions, we should use HANDLER_PROTO in the
declaration.  See the other handlers.

>   * Authors:
>   *   Florian octo Forster <octo at verplant.org>
> - *   kevin brintnall <kbrint at rufus.net>
> + *   Kevin Brintnall <kbrint at rufus.net>

Please leave as-is.

> +static int handle_request_fetch (listen_socket_t *sock, /* {{{ */
> +    char *buffer, size_t buffer_size)
> +{
>
> ...
>
> +  status = flush_file (file);

Maybe we should check the start/end times, compared to the
cache_item->last_flush_time.  When calling "FETCH" on really old values,
flush would be unnecessary.  Can we do it without knowing each file's rrd
step value?

> +#define SSTRCAT(buffer,str,buffer_fill) do { \
> +    size_t str_len = strlen (str); \
> +    if ((buffer_fill + str_len) > sizeof (buffer)) \
> +      str_len = sizeof (buffer) - buffer_fill; \
> + ...

Why not strlcat?

> +    for (i = 0; i < ds_cnt; i++)
> +    {
> +      snprintf (tmp, sizeof (tmp), " %0.10e", *data_ptr);
> +      tmp[sizeof (tmp) - 1] = 0;

snprintf always NULL terminates, the tmp[]=0 is superfluous.

Docs patch for the "FLUSH" command?

-- 
 kevin brintnall =~ /kbrint at rufus.net/