[collectd] [PATCH] Linux ACPI thermal zone plugin

[Florian Forster]

Hi Michał,

On Mon, Jun 09, 2008 at 08:33:23PM +0200, Michał Mirosław wrote:
> Here's temperature monitoring plugin using Linux ACPI thermal zone
> data from /sys/class/thermal/ or /proc/acpi/thermal_zone/.

thank you very much for your patch :)

I have a few (minor) requests for changes though ;)
- configure.in: You set `plugin_thermal' to `yes' only if the script
  runs under Linux, but later you enable the module unconditionally. So
  instead of
  AC_PLUGIN([thermal],     [yes],                [Linux ACPI thermal zone statistics])
  please use
  AC_PLUGIN([thermal],     [$plugin_thermal],    [Linux ACPI thermal zone statistics])
- src/thermal.c: Your name is not in the copyright notice at the top of
  the file.. Also, the name of the file stated there, `src/battery.c' is
  incorrect..
- You implemented the configuration directive `Devices'. Since one needs
  to repeat that directive for each device that is to be collected/
  ignored we use the singular, `Device', in similar places of other
  plugins. Could you change that to be more consistent, please?
- You use `strcpy' in some places. I see that these uses are harmless,
  but could you please use `sstrncpy' (from "common.h") anyway? This way
  next time I look at the plugin I don't have to manually check if the
  buffer really is big enough in any case ;)
  (I know this is used in a lot of other plugins still, but I'm creating
  a patch right now to eliminate all uses of `strcpy'.)

Thanks again for your code :)
Best regards,
-octo
-- 
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/20080611/8dece4a9/attachment.pgp