[collectd] [PATCH] Madwifi plugin

[Florian Forster]

Hi Ondrej,

On Fri, Jul 17, 2009 at 11:13:31PM +0200, Ondrej Zajicek wrote:
> Here is a patch adding plugin for collecting statistics of Madwifi
> wireless driver.

thank you very much for your patch :)

> diff -uprN collectd-4.6.3/configure.in collectd-4.6.3-mod/configure.in

I was able to rebase the patch on the master branch without any
problems. If you want I can push that commit for you to use as a base
for further changes.

> +#include "madwifi.h"

You're shipping that header file with structure declarations. Is it
possible to use already existing header files to get those declarations?
They are later used for `ioctl', so I'd rather use the systems
declarations to avoid problems should those structures ever be changed..

> +static inline void
> +macaddr_to_str(char *buf, int bufsize, const uint8_t mac[IEEE80211_ADDR_LEN])
> +{
> +	snprintf (buf, bufsize, "%02x:%02x:%02x:%02x:%02x:%02x",
> +		mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]);
> +}

Please use the correct format string for uint8_t here. It's
  snprintf (buf, bufsize,
    "%02"PRIx8":%02"PRIx8":%02"PRIx8":%02"PRIx8":%02"PRIx8":%02"PRIx8,
    mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]);
Also, `bufsize' should be a size_t.

> --- collectd-4.6.3/src/types.db	2009-06-02 11:17:47.000000000 +0200
> +++ collectd-4.6.3-mod/src/types.db	2009-07-17 21:57:50.000000000 +0200

You are adding *a lot* of new types. This is not the usual way in which
data is structured in collectd, see [0].

Please use
  type = "if_frames"; type_instance = "tx-noack";
instead of
  type = "ath_hw_tx_noack"; type_instance = NULL;
or
  type = "if_errors"; type_instance = "rx-badcrypt";
instead of
  type = "ath_hw_rx_badcrypt"; type_instance = NULL;
and so on.

In short, the `type' should always describe *what* is being counted/
collected/... and the `type_instance' should differenciate between
multiple values of the same nature (== type).

Best regards,
-octo

[0] <http://collectd.org/wiki/index.php/Naming_schema>
-- 
Florian octo Forster
Hacker in training
GnuPG: 0x91523C3D
http://verplant.org/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://mailman.verplant.org/pipermail/collectd/attachments/20090719/a4e441aa/attachment.pgp