Re: [PATCH] Avoid hard coded buffer lengths in rrd_client.c.

Hi Kevin,

Nov 16 kevin brintnall wrote:

> The buffer length for command buffers should be controlled by a single
> #define.
>
> I put this into "rrd_client.h"..  Since that header is exposed as part of
> the API perhaps we should consider renaming it to "RRD_CMD_MAX" or
> somesuch?  Or we could put it into "rrd_tool.h".  Thoughts?

I have added the patch and renamed CMD_MAX to RRD_CMD_MAX. I think
rrd_client.h is a good place as it will be primarily useful to
people using the rrd_client.c ..

cheers
tobi

>
> ---
>  src/rrd_client.c |   20 ++++++++++----------
>  src/rrd_client.h |    2 ++
>  src/rrd_daemon.c |    2 --
>  3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/src/rrd_client.c b/src/rrd_client.c
> index 870a27b..893e526 100644
> --- a/src/rrd_client.c
> +++ b/src/rrd_client.c
> @@ -304,7 +304,7 @@ static int buffer_add_string (const char *str, /* {{{ */
>  static int buffer_add_value (const char *value, /* {{{ */
>      char **buffer_ret, size_t *buffer_size_ret)
>  {
> -  char temp[4096];
> +  char temp[CMD_MAX];
>
>    if (strncmp (value, "N:", 2) == 0)
>      snprintf (temp, sizeof (temp), "%lu:%s",
> @@ -319,7 +319,7 @@ static int buffer_add_value (const char *value, /* {{{ */
>  static int buffer_add_ulong (const unsigned long value, /* {{{ */
>      char **buffer_ret, size_t *buffer_size_ret)
>  {
> -  char temp[4096];
> +  char temp[CMD_MAX];
>
>    snprintf (temp, sizeof (temp), "%lu", value);
>    temp[sizeof (temp) - 1] = 0;
> @@ -372,7 +372,7 @@ static int response_read (rrdc_response_t **ret_response) /* {{{ */
>    rrdc_response_t *ret = NULL;
>    int status = 0;
>
> -  char buffer[4096];
> +  char buffer[CMD_MAX];
>    char *buffer_ptr;
>
>    size_t i;
> @@ -713,7 +713,7 @@ int rrdc_disconnect (void) /* {{{ */
>  int rrdc_update (const char *filename, int values_num, /* {{{ */
>  		const char * const *values)
>  {
> -  char buffer[4096];
> +  char buffer[CMD_MAX];
>    char *buffer_ptr;
>    size_t buffer_free;
>    size_t buffer_size;
> @@ -775,7 +775,7 @@ int rrdc_update (const char *filename, int values_num, /* {{{ */
>
>  int rrdc_flush (const char *filename) /* {{{ */
>  {
> -  char buffer[4096];
> +  char buffer[CMD_MAX];
>    char *buffer_ptr;
>    size_t buffer_free;
>    size_t buffer_size;
> @@ -829,7 +829,7 @@ int rrdc_flush (const char *filename) /* {{{ */
>
>  rrd_info_t * rrdc_info (const char *filename) /* {{{ */
>  {
> -  char buffer[4096];
> +  char buffer[CMD_MAX];
>    char *buffer_ptr;
>    size_t buffer_free;
>    size_t buffer_size;
> @@ -930,7 +930,7 @@ rrd_info_t * rrdc_info (const char *filename) /* {{{ */
>
>  time_t rrdc_last (const char *filename) /* {{{ */
>  {
> -  char buffer[4096];
> +  char buffer[CMD_MAX];
>    char *buffer_ptr;
>    size_t buffer_free;
>    size_t buffer_size;
> @@ -991,7 +991,7 @@ time_t rrdc_last (const char *filename) /* {{{ */
>
>  time_t rrdc_first (const char *filename, int rraindex) /* {{{ */
>  {
> -  char buffer[4096];
> +  char buffer[CMD_MAX];
>    char *buffer_ptr;
>    size_t buffer_free;
>    size_t buffer_size;
> @@ -1064,7 +1064,7 @@ int rrdc_create (const char *filename, /* {{{ */
>      int argc,
>      const char **argv)
>  {
> -  char buffer[4096];
> +  char buffer[CMD_MAX];
>    char *buffer_ptr;
>    size_t buffer_free;
>    size_t buffer_size;
> @@ -1149,7 +1149,7 @@ int rrdc_fetch (const char *filename, /* {{{ */
>      char ***ret_ds_names,
>      rrd_value_t **ret_data)
>  {
> -  char buffer[4096];
> +  char buffer[CMD_MAX];
>    char *buffer_ptr;
>    size_t buffer_free;
>    size_t buffer_size;
> diff --git a/src/rrd_client.h b/src/rrd_client.h
> index e203d7c..b6e0af0 100644
> --- a/src/rrd_client.h
> +++ b/src/rrd_client.h
> @@ -49,6 +49,8 @@
>  	typedef unsigned long long int 	uint64_t;
>  #endif
>
> +/* max length of socket command or response */
> +#define CMD_MAX 4096
>
>  #ifndef RRDCACHED_DEFAULT_ADDRESS
>  # define RRDCACHED_DEFAULT_ADDRESS "unix:/tmp/rrdcached.sock"
> diff --git a/src/rrd_daemon.c b/src/rrd_daemon.c
> index 0586a8d..18d00c8 100644
> --- a/src/rrd_daemon.c
> +++ b/src/rrd_daemon.c
> @@ -219,8 +219,6 @@ typedef struct {
>    size_t files_num;
>  } journal_set;
>
> -/* max length of socket command or response */
> -#define CMD_MAX 4096
>  #define RBUF_SIZE (CMD_MAX*2)
>
>  /*
>

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