[collectd] (maybe silly) Question about get_kstat

Marco Chiappero marco at absence.it
Thu Mar 19 18:59:33 CET 2009


Florian Forster ha scritto:
> Hi Marco,

Hi :)

> looks very good, thank you :) I've added the required build-system
> changes and made some changes, mostly shuffled the preprocessor stuff
> around a bit and changed some comments.

Good!

> I think I've fixed a Solaris bug, too:
> 
> -if ((knp = (kstat_named_t *) kstat_data_lookup (ksp, "boot_time")) != NULL)
> +knp = (kstat_named_t *) kstat_data_lookup (ksp, "boot_time");
> +if (knp == NULL)

Oh, thank you for noticing! Firstly I wrote if( true && true ) { .. 
return 0; }, then switched to return -1 but I forgot to fully change in 
if( false || false ). Sorry about that, didn't check the code enough.
Your code is fine, or this one too:

...
kstat_read(kc, ksp, NULL);
if (( knp = (kstat_named_t *) kstat_data_lookup (ksp, "boot_time")) == 
NULL )
...

As you like...

> By the way, the boot-time can be read under Linux from `/proc/stat'.
> Maybe that'd simplify the plugin further..?

Didn't know that, but I already thought about removing the fopen/fclose 
stuff and explicity calculating the boot time when calling uptime_init 
as curr_time - uptime.
I attached the code mainly for a review, hoping for some testing about 
the Solaris/*BSD part.
By the way, thank you for the hint!

> I've added the plugin to the master branch, it'd be great if you could
> base further changes on that. :)

Sure, I'm going to change the linux related code.
Thank you very much for the inclusion.

> Oh, right, I wanted to do that last weekend but then got stuck with the
> web stuff :/

The web stuff is indeed more important. :)

Regards,
Marco



More information about the collectd mailing list