[dev.icinga.com #2197] Revamp the detection of embedded perl usage directive "# icinga: +epn"

[icinga-migration]

This issue has been migrated from Redmine: https://dev.icinga.com/issues/2197

Created by mfriedrich on 2011-12-14 08:14:40 +00:00

Assignee: mfriedrich
Status: Resolved (closed on 2012-07-31 19:27:34 +00:00)
Target Version: 1.8
Last Update: 2012-08-21 15:34:58 +00:00 (in Redmine)

---

needed tests

Revision: 1854
          http://nagios.svn.sourceforge.net/nagios/?rev=1854&view=rev
Author:   ageric
Date:     2011-12-14 00:52:30 +0000 (Wed, 14 Dec 2011)
Log Message:
-----------
core: Revamp file_uses_embedded_perl()

Primaily we do this to get rid of the non-reentrant strtok() call.
Since we only handle one type of directive anyway, we can easily
optimize for that simple case and be done much quicker. If we add
more directives later we handle that then.

While we're at it, we get rid of some superfluous variables to
reduce the memory footprint of Nagios somewhat and generally clean
up the code.

We also remove the completely insane indentation depth that used
to be there that was exceedingly difficult to keep track of.

To top it all off, we increase the buffer we're reading in to. A
lot of the Perl plugins on Nagios-exchange use lines longer than
80 characters, so if we fgets() such a line the "nagios directives
must be within first 10 lines" is no longer true. None of the (few)
plugins I checked has any top-level comments spanning more than
256 chars though, and since that's a nice round number (for stuff
counting things in binary anyways) we stick with that.

Signed-off-by: Andreas Ericsson 

Modified Paths:
--------------
    nagioscore/trunk/base/utils.c

Modified: nagioscore/trunk/base/utils.c
===================================================================
--- nagioscore/trunk/base/utils.c   2011-12-12 14:28:08 UTC (rev 1853)
+++ nagioscore/trunk/base/utils.c   2011-12-14 00:52:30 UTC (rev 1854)
@@ -2011,7 +2011,7 @@
    if(fcntl(lockfile, F_SETLK, &lock) < 0) {
        if(errno == EACCES || errno == EAGAIN) {
            fcntl(lockfile, F_GETLK, &lock);
-           logit(NSLOG_RUNTIME_ERROR, TRUE, "Lockfile '%s' looks like its already held by another instance of Nagios (PID %d).  Bailing out...", lock_file, (int)lock.l_pid);
+           logit(NSLOG_RUNTIME_ERROR, TRUE, "Lockfile '%s' looks like its already held by another instance of op5 Monitor (PID %d).  Bailing out...", lock_file, (int)lock.l_pid);
            }
        else
            logit(NSLOG_RUNTIME_ERROR, TRUE, "Cannot lock lockfile '%s': %s. Bailing out...", lock_file, strerror(errno));
@@ -2615,8 +2615,6 @@
    }


-
-
 /* creates external command file as a named pipe (FIFO) and opens it for reading (non-blocked mode) */
 int open_command_file(void) {
    struct stat st;
@@ -2639,7 +2637,7 @@
        /* create the external command file as a named pipe (FIFO) */
        if((result = mkfifo(command_file, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP)) != 0) {

-           logit(NSLOG_RUNTIME_ERROR, TRUE, "Error: Could not create external command file '%s' as named pipe: (%d) -> %s.  If this file already exists and you are sure that another copy of Nagios is not running, you should delete this file.\n", command_file, errno, strerror(errno));
+           logit(NSLOG_RUNTIME_ERROR, TRUE, "Error: Could not create external command file '%s' as named pipe: (%d) -> %s.  If this file already exists and you are sure that another copy of op5 Monitor is not running, you should delete this file.\n", command_file, errno, strerror(errno));
            return ERROR;
            }
        }
@@ -3114,73 +3112,55 @@

 /* checks to see if we should run a script using the embedded Perl interpreter */
 int file_uses_embedded_perl(char *fname) {
-   int use_epn = FALSE;
-#ifdef EMBEDDEDPERL
+#ifndef EMBEDDEDPERL
+   return FALSE;
+#else
+   int line, use_epn = FALSE;
    FILE *fp = NULL;
-   char line1[80] = "";
-   char linen[80] = "";
-   int line = 0;
-   char *ptr = NULL;
-   int found_epn_directive = FALSE;
+   char buf[256] = "";

-   if(enable_embedded_perl == TRUE) {
+   if(enable_embedded_perl != TRUE)
+       return FALSE;

-       /* open the file, check if its a Perl script and see if we can use epn  */
-       fp = fopen(fname, "r");
-       if(fp != NULL) {
+   /* open the file, check if its a Perl script and see if we can use epn  */
+   fp = fopen(fname, "r");
+   if(fp == NULL)
+       return FALSE;

-           /* grab the first line - we should see Perl */
-           fgets(line1, 80, fp);
+   /* grab the first line - we should see Perl. go home if not */
+   if (fgets(buf, 80, fp) == NULL || strstr(buf, "/bin/perl") == NULL) {
+       fclose(fp);
+       return FALSE;
+   }

-           /* yep, its a Perl script... */
-           if(strstr(line1, "/bin/perl") != NULL) {
+   /* epn directives must be found in first ten lines of plugin */
+   for(line = 1; line < 10; line++) {
+       if(fgets(buf, sizeof(buf) - 1, fp) == NULL)
+           break;

-               /* epn directives must be found in first ten lines of plugin */
-               for(line = 1; line < 10; line++) {
+       buf[sizeof(buf) - 1] = '\0';

-                   if(fgets(linen, 80, fp)) {
+       /* skip lines not containing nagios 'epn' directives */
+       if(strstr(buf, "# nagios:")) {
+           char *p;
+           p = strstr(buf + 8, "epn");
+           if (!p)
+               continue;

-                       /* line contains Nagios directives */
-                       if(strstr(linen, "# nagios:")) {
-
-                           ptr = strtok(linen, ":");
-
-                           /* process each directive */
-                           for(ptr = strtok(NULL, ","); ptr != NULL; ptr = strtok(NULL, ",")) {
-
-                               strip(ptr);
-
-                               if(!strcmp(ptr, "+epn")) {
-                                   use_epn = TRUE;
-                                   found_epn_directive = TRUE;
-                                   }
-                               else if(!strcmp(ptr, "-epn")) {
-                                   use_epn = FALSE;
-                                   found_epn_directive = TRUE;
-                                   }
-                               }
-                           }
-
-                       if(found_epn_directive == TRUE)
-                           break;
-                       }
-
-                   /* EOF */
-                   else
-                       break;
-                   }
-
-               /* if the plugin didn't tell us whether or not to use embedded Perl, use implicit value */
-               if(found_epn_directive == FALSE)
-                   use_epn = (use_embedded_perl_implicitly == TRUE) ? TRUE : FALSE;
-               }
-
+           /*
+            * we found it, so close the file and return
+            * whatever it shows. '+epn' means yes. everything
+            * else means no
+            */
            fclose(fp);
+           return *(p - 1) == '+' ? TRUE : FALSE;
            }
        }
-#endif

-   return use_epn;
+   fclose(fp);
+
+   return use_embedded_perl_implicitly;
+#endif
    }


@@ -3735,6 +3715,8 @@
    unsigned int rand_seed = 0;
    int randnum = 0;

+   return 0; /* op5 users get their updates elsewhere */
+
    time(&current_time);

    /*
@@ -4243,9 +4225,9 @@
    passive_host_checks_are_soft = DEFAULT_PASSIVE_HOST_CHECKS_SOFT;

    use_large_installation_tweaks = DEFAULT_USE_LARGE_INSTALLATION_TWEAKS;
-   enable_environment_macros = TRUE;
-   free_child_process_memory = -1;
-   child_processes_fork_twice = -1;
+   enable_environment_macros = FALSE;
+   free_child_process_memory = FALSE;
+   child_processes_fork_twice = FALSE;

    additional_freshness_latency = DEFAULT_ADDITIONAL_FRESHNESS_LATENCY;


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Changesets

2012-07-06 16:33:01 +00:00 by mfriedrich 9707f13

core: revamp the detection of embedded perl usage directive "# icinga: +epn" (Andreas Ericsson) #2197

patch is revamping the detection of the first 10 lines containing the
"#icinga: +epn" line.

previously, the core would have read only the first 80 characters from
each line via fgets, which could be to less. the inner handling is
changed to 256 chars, which should be sufficient. removing the strtok on
':' is also a good idea, as this can be done via the strstr pointer
match on 'epn' as well, calculating the return value based on the
character ('+' means embedded perl enabled).

refs #2197

2012-08-19 17:33:30 +00:00 by mfriedrich 6009a11

core: revamp the detection of embedded perl usage directive "# icinga: +epn" (Andreas Ericsson) #2197

patch is revamping the detection of the first 10 lines containing the
"#icinga: +epn" line.

previously, the core would have read only the first 80 characters from
each line via fgets, which could be to less. the inner handling is
changed to 256 chars, which should be sufficient. removing the strtok on
':' is also a good idea, as this can be done via the strstr pointer
match on 'epn' as well, calculating the return value based on the
character ('+' means embedded perl enabled).

refs #2197

Conflicts:
	Changelog

2012-08-21 15:35:04 +00:00 by mfriedrich eeb8cde

core: fix compiling with embedded perl #2197

i fucked up, sorry. buy me more beer for proper code and tests :p

refs #2197

2012-08-21 15:36:25 +00:00 by mfriedrich b513751

core: fix compiling with embedded perl #2197

i fucked up, sorry. buy me more beer for proper code and tests :p

refs #2197

---

Relations:

• relates #2380

[icinga-migration]

Updated by mfriedrich on 2012-04-20 12:58:40 +00:00

• Target Version set to 1.8

[icinga-migration]

Updated by mfriedrich on 2012-05-13 17:30:01 +00:00

• Status changed from Feedback to Assigned
• Assigned to set to mfriedrich

could be related to #2380

[icinga-migration]

Updated by mfriedrich on 2012-07-06 16:28:38 +00:00

• Category set to Embedded Perl

patch is revamping the detection of the first 10 lines containing the "#icinga: +epn" line.

previously, the core would have read only the first 80 characters from each line via fgets, which could be to less. the inner handling is changed to 256 chars, which should be sufficient. removing the strtok on ':' is also a good idea, as this can be done via the strstr pointer match on 'epn' as well, calculating the return value based on the character ('+' means embedded perl enabled).

[icinga-migration]

Updated by mfriedrich on 2012-07-06 16:32:55 +00:00

• Subject changed from Revamp file_uses_embedded_perl() to Revamp the detection of embedded perl usage directive "# icinga: +epn"

[icinga-migration]

Updated by mfriedrich on 2012-07-20 18:14:43 +00:00

• Status changed from Assigned to Feedback
• Done % changed from 0 to 90

[icinga-migration]

Updated by mfriedrich on 2012-07-31 19:27:34 +00:00

• Status changed from Feedback to Resolved
• Done % changed from 90 to 100

[icinga-migration]

Updated by mfriedrich on 2012-08-21 15:34:58 +00:00

build with embedded perl was wrong, did not test properly with $ sudo apt-get install libperl-dev ; ./configure --enable-embedded-perl