[collectd] [PATCH 2/5] sigrok: bare bones plugin

Dan Fandrich dan at coneharvesters.com
Mon Jul 22 21:35:48 CEST 2013


On Mon, Jul 22, 2013 at 06:21:18PM +0200, Bert Vermeulen wrote:
> +static int plugin_config(const char *key, const char *value)
> +{
> +
> +	if (!strcmp(key, "loglevel"))
> +		loglevel = atoi(value);
> +	else if (!strcmp(key, "conn"))
> +		conn = strdup(value);

This will leak memory if the config file defines these elements more than once.

> +	else if (!strcmp(key, "serialcomm"))
> +		serialcomm = strdup(value);

Here too.

> +	else
> +		return 1;
> +
> +	return 0;
> +}
> +
> +static int plugin_init(void)
> +{
> +	int ret;
> +
> +	sr_log_callback_set(cd_logger, NULL);
> +	sr_log_loglevel_set(loglevel);
> +
> +	if ((ret = sr_init(&sr_ctx)) != SR_OK) {
> +		INFO("Failed to initialize libsigrok: %s.", sr_strerror(ret));
> +		return 1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int plugin_read(user_data_t *ud)
> +{
> +
> +	return 0;
> +}

Personally, I don't see the need to split this patch up into several pieces. A
skeleton by itself isn't very useful to review.

> +
> +static int plugin_shutdown(void)
> +{
> +	if (conn)
> +		free(conn);
> +	if (serialcomm)
> +		free(serialcomm);

The if check for NULL is not needed, as free is guaranteed to be able to handle it.

> +
> +	sr_exit(sr_ctx);
> +
> +	return 0;
> +}

>>> Dan



More information about the collectd mailing list