[collectd] PATCH: ipmi

[Peter Holik]

> Could you maybe separate this into two patches? One for the notification
> stuff and one for the rest?

OK, comming son

> What does the global `c_ipmi_sensor_removed' variable do? It's set to
> one in `sensor_list_remove' and then never reset to anything else.. Do
> you want to send `ADD' notifications only after a `REMOVE' has been
> reported? Wouldn't it make more sense to do something like:
> -- 8< --
>   static int c_ipmi_init_in_progress;
>   ...
>   sensor_list_add ()
>   {
>     if (c_ipmi_nofiy_add && (c_ipmi_init_in_progress == 0))
>       /* send_notification */
>   }
>   ...
>   c_ipmi_init ()
>   {
>     /* Don't send `ADD' notifications during startup */
>     c_ipmi_init_in_progress = 1 + (10 / interval_g);
>   }
>   ...
>   c_ipmi_read ()
>   {
>     sensor_list_read_all ();
>
>     if (c_ipmi_init_in_progress > 0)
>       c_ipmi_init_in_progress--;
>     else
>       c_ipmi_init_in_progress = 0;
>   }
> -- >8 --

Yes, something like your idea was my first try, but entity_sensor_update_handler with
IPMI_ADDED was called long after init finished.


c_ipmi_init_in_progress = 1 + (10 / interval_g);

why 10 / interval_g ?

> You use this fancy C99 syntax to initialize the `notification_t' struct:
> -- 8< --
>   notification_t n = { .severity = NOTIF_WARNING,
>     .time = time(NULL),
>     .plugin = "ipmi",
>     .plugin_instance = "",
>     .meta = NULL };
> -- >8 --
> I agree that his sytnax is nice and handy, but unfortunately there are
> popular compilers which can't cope with this syntax - I've stumbled upon
> this problem with the Sun C compiler, for example. So could you change
> this to the old
>   notification_t n = { NOTIF_WARNING, time(NULL), "", "", "ipmi", "", "", "", NULL };
> syntax please?

OK

> Last but not least: The entire C file is indented with spaces. Could you change
> your code to use spaces rather than tabs, please?

i will try

cu Peter