[Nagiosplug-devel] Check_disk plugin unified diff

[Thomas Guyot-Sionnest]

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

On 06/11/08 06:17 PM, Erik Welch wrote:
> --- ./check_disk.c.new  Thu Nov  6 10:58:28 2008
> +++ /tmp/nagios-plugins-1.4.12/plugins/check_disk.c     Wed May  7 03:02:42 2008
> @@ -190,14 +190,14 @@
>    bindtextdomain (PACKAGE, LOCALEDIR);
>    textdomain (PACKAGE);
>   
> +  mount_list = read_file_system_list (0);
> +
>    /* Parse extra opts if any */
>    argv = np_extra_opts (&argc, argv, progname);
>   
>    if (process_arguments (argc, argv) == ERROR)
>      usage4 (_("Could not parse arguments"));
>   
> -  mount_list = read_file_system_list (0);
> -
>    /* If a list of paths has not been selected, find entire
>       mount list and create list of paths
>     */
> @@ -639,7 +639,7 @@
>        }
>        se->group = group;
>        set_all_thresholds(se);
> -      /* np_set_best_match(se, mount_list, exact_match); */
> +      np_set_best_match(se, mount_list, exact_match);
>        stat_path(se);
>        path_selected = TRUE;
>        break;
> @@ -747,7 +747,7 @@
>   
>        fnd = FALSE;
>        path_selected = TRUE;
> -      /* np_set_best_match(path_select_list, mount_list, exact_match); */
> +      np_set_best_match(path_select_list, mount_list, exact_match);
>        cflags = default_cflags;
>   
>        break;

Thanks, although next time please send it as an attachment as important
formating is lost when you send it inline. Making a forward patch would
be appreciated too ;).

Unfortunately I'm afraid this will break multiple options that require a
populated mount_list structure during parsing (including -p in some
circumstances I think). Also when you said that the result of
np_set_best_match() is unused, did you realize that path_select_list is
declared globally and np_set_best_match() modify data inside that
structure? Again, there's some other behaviours that are likely broken.

And as expected, applying your path breaks some of the built-in tests (5
tests to be more precise):

Failed Test    Stat Wstat Total Fail  List of Failed
-
-------------------------------------------------------------------------------
t/check_disk.t    5  1280    78    5  66 72-74 76

I'm not very used to automount environments (didn't use one for years),
so unless you can find a way to fix it otherwise I'm affraid I won't be
of any help. Maybe you could try re-reading the mountlist after the stat
while parsing -p; i.e.:

      stat_path(se);
      mount_list = read_file_system_list (0);
      np_set_best_match(se, mount_list, exact_match);

Be sure to comment well why you're doing that though. It would also be
interesting to see the performance difference... If the impact is too
high it might be worth making this extra read_file_system_list()
optional, triggered by another switch.

Hope this helps, and thanks for contributing your patches.

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

iD8DBQFJE69T6dZ+Kt5BchYRAkqaAKDj3+OetqY/k5qosD0LcDz1vNyKTgCePZCs
xjb+ioI4rLLyWSqrU2b9Oac=
=o3pt
-----END PGP SIGNATURE-----