[Nagiosplug-devel] Patch for persistent data on plugins

[Thomas Guyot-Sionnest]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 16/09/09 08:02 AM, Ton Voon wrote:
> 
> On 16 Sep 2009, at 12:39, Alain Williams wrote:
> 
>> Please find attached a new version of the patch that I submitted a
>> week ago.
>>
>> This provides an back end neutral mechanism for persistant data
>> storage for plugins written in C.
>> This follows the discussion that I started by trying to get accepted a
>> patch for check_procs.
>>
>> I must confess to being disappointed that there has not been any
>> comment about this. It does
>> seem hard to help the nagios plugins project.
>>
>> My original email (please read):
>>
>> http://sourceforge.net/mailarchive/message.php?msg_name=20090909204742.GI29402%40phcomp.co.uk
>>
>> The patch is against nagios-plugins-trunk-200909091200.
> 
> Sorry, I didn't see the email until today (because someone replied to
> the thread with a different subject - I know, bad excuse).
> 
> I'll take a look later

My take on this...:

1. Tests should use libtap, and lets remove those goto's before someone
else see them ;)

2. Doc should go in the docbook, not a plain text file (if there are too
technical portions those could be be code comments

3. A plugin shouldn't share state file for multiple different
invocations. Plugins may run in parallel so this is asking for trouble
(one invocation overwriting another plugin's data). That also call for a
much simpler API and plugin code.

4. Should we write a temp file first and move it over instead of
truncating it? IMHO the former is much better is the file is not opened
and saver at once...

5. time.h is already included in common.h

6. Binary strings should go with string length - they may contain nulls.

7. Why do np_state_add_vcomment() needs first + comment_array? Why a
standalone array won't work??

8. I don't think the descripting/comments should go in there. They could
be added manually or trough a serialization library, but in most cases
the pluginj's verbose mode should be enough to debug.

9. On some unix unlink() may damage file system if the path is a
directory and the plugin is run as root. Any sysadmin ending up in that
situation should be shot in the head, but it's an easy check...

10. DIR_DEFAULT should come from configure with an option to change it.
The default should likely be PREFIX/var/rw or something like that
(nagios temp path... you get the idea)

12. I'd suggest having all files prefixed with "np_" instead of using
some sort of heuristic to determine whenever we should add
"nagios-plugin_" (bonus point: making prefix a configure parameter).

13. I like the ascii header, but I'd use a more compact version. This
ensure state files are as small as possible, preventing waste (very
useful when a tmpfs is used for storage and helping usage of tail files
on fs that support it).

14. There's no additional tests in check_procs.t

- --
Thomas
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFKsdHA6dZ+Kt5BchYRAkPbAJ9xjRNugz8UVrgggrIOy+g7hIku5wCfRzAO
wdlt5+/YrVHkdnwApHabSuw=
=kWYE
-----END PGP SIGNATURE-----