[icinga-checkins] icinga.org: icinga-core/cmaser/fixes: revert buggy my_fcopy hacks for nebmods

git at icinga.org git at icinga.org
Sat Jul 3 22:41:49 CEST 2010


Module: icinga-core
Branch: cmaser/fixes
Commit: 0aa908d1f577985a0e97f6c2eac07f8472548b77
URL:    https://git.icinga.org/?p=icinga-core.git;a=commit;h=0aa908d1f577985a0e97f6c2eac07f8472548b77

Author: Michael Friedrich <michael.friedrich at univie.ac.at>
Date:   Fri Jun 25 12:55:03 2010 +0200

revert buggy my_fcopy hacks for nebmods

-* core: base/nebmods.c: Replace local file-copy hack with my_fcopy() (Andreas Ericsson)
-* core: base/utils.c: Refactor my_fcopy() (Andreas Ericsson)

those copy stuff causes more than one neb module (one with
config_file and one just with a param, like idomod and
mk_livestatus) to fail.
also it depends on the order of the modules in icinga.cfg
which is really bad.

if ordered idomod|\n|mklivestatus => mklivestatus now has
the param of idomod, idomod has none and fails to load

if ordered mklivestatus|\n|idomod, idomod gets loaded
correctly, but mklivestatus fails not getting the socket
as param.

since 1.0.2 is coming soon, changes are reverted, tested ok.
it will need some further investigations to finally resolve
those issues on a dev branch.

kudos to Lara for reporting :)

fixes #535

---

 Changelog      |    2 -
 THANKS         |    1 +
 base/nebmods.c |   24 ++++++++++--
 base/utils.c   |  107 ++++++++++++--------------------------------------------
 4 files changed, 44 insertions(+), 90 deletions(-)

diff --git a/Changelog b/Changelog
index 6f21dcb..518a927 100644
--- a/Changelog
+++ b/Changelog
@@ -68,9 +68,7 @@ FIXES
 * core: header files: Remove dead prototypes (Andreas Ericsson)
 * core: remove grab_contactgroup_macros() and its prototype (Andreas Ericsson)
 * core: base/commands.c: Fix error path of opening checkresult files (Andreas Ericsson)
-* core: base/nebmods.c: Replace local file-copy hack with my_fcopy() (Andreas Ericsson)
 * core: fix open() error checking in move_check_result_to_queue() (Andreas Ericsson)
-* core: base/utils.c: Refactor my_fcopy() (Andreas Ericsson)
 * core: xdata/xpddefault.c: Close perfdata files if fd's are >= 0 (Andreas Ericsson)
 * core: fix xpddefault_{host,service}_perfdata_file_pipe not set properly on configuration re-read
 * core: fix SIGSEGV in checks.c on Solaris (Torsten Huebler)
diff --git a/THANKS b/THANKS
index 202072c..d57cfb2 100644
--- a/THANKS
+++ b/THANKS
@@ -31,6 +31,7 @@ in various ways.  If we missed your name, let us know.
 * Derrick Bennett
 * Chris Bensend
 * Kevin Benton
+* Lara Berdelsmann
 * Gary Berger
 * Tom Bertelson
 * Joel Berry
diff --git a/base/nebmods.c b/base/nebmods.c
index 4fc1f1e..697e95a 100644
--- a/base/nebmods.c
+++ b/base/nebmods.c
@@ -168,6 +168,11 @@ int neb_load_module(nebmodule *mod){
 	int *module_version_ptr=NULL;
 	char *output_file=NULL;
 	int result=OK;
+        int dest_fd=-1;
+        int source_fd=-1;
+        char buffer[MAX_INPUT_BUFFER]={0};
+        int bytes_read=0;
+
 
 	if(mod==NULL || mod->filename==NULL)
 		return ERROR;
@@ -196,11 +201,22 @@ int neb_load_module(nebmodule *mod){
 
 	/* open a temp file for copying the module */
 	asprintf(&output_file,"%s/nebmodXXXXXX",temp_path);
-	if (my_fcopy(mod->filename, output_file) == ERROR) {
-		logit(NSLOG_RUNTIME_ERROR,FALSE,"Error: Failed to safely copy module '%s'. The module will not be loaded\n", mod->filename);
-		free(output_file);
+        if((dest_fd=mkstemp(output_file))==-1){
+                logit(NSLOG_RUNTIME_ERROR,FALSE,"Error: Could not safely copy module '%s'.  The module will not be loaded: %s\n",mod->filename,strerror(errno));
 		return ERROR;
-	}
+		}
+        /* open module file for reading and copy it */
+        if((source_fd=open(mod->filename,O_RDONLY,0644))>0){
+                while((bytes_read=read(source_fd,buffer,sizeof(buffer)))>0)
+                        write(dest_fd,buffer,bytes_read);
+                close(source_fd);
+                close(dest_fd);
+                }
+        else{
+                logit(NSLOG_RUNTIME_ERROR,FALSE,"Error: Could not safely copy module '%s'.  The module will not be loaded: %s\n",mod->filename,strerror(errno));
+                return ERROR;
+                }
+
 	/* load the module (use the temp copy we just made) */
 #ifdef USE_LTDL
 	mod->module_handle=lt_dlopen(output_file);
diff --git a/base/utils.c b/base/utils.c
index 2c940ac..c64f113 100644
--- a/base/utils.c
+++ b/base/utils.c
@@ -3387,12 +3387,10 @@ int my_rename(char *source, char *dest){
 
 /* copies a file */
 int my_fcopy(char *source, char *dest){
+	char buffer[MAX_INPUT_BUFFER]={0};
 	int source_fd=-1;
 	int dest_fd=-1;
-        int rd_result = 0, wr_result = 0;
-        off_t tot_written = 0, tot_read = 0, buf_size = 0;
-        char *buf;
-        struct stat st;
+	int bytes_read=0;
 
 	/* make sure we have something */
 	if(source==NULL || dest==NULL)
@@ -3401,91 +3399,32 @@ int my_fcopy(char *source, char *dest){
 	/* unlink destination file first (not doing so can cause problems on network file systems like CIFS) */
 	unlink(dest);
 
-        /* open source file for reading */
-        if((source_fd=open(source,O_RDONLY,0644)) < 0){
-                logit(NSLOG_RUNTIME_ERROR,TRUE,"Error: Unable to open file '%s' for reading: %s\n",source,strerror(errno));
-                return ERROR;
-        }
- 
-        /*
-         * find out how large the source-file is so we can be sure
-         * we've written all of it
-         */
-        if (fstat(source_fd, &st) < 0) {
-                logit(NSLOG_RUNTIME_ERROR,TRUE,"Error: Unable to stat '%s' for my_fcopy(): %s\n", source, strerror(errno));
-                close(source_fd);
-                return ERROR;
-        }
- 
-        /*
-         * If the file is huge, read it and write it in chunks.
-         * This value (128K) is the result of "pick-one-at-random"
-         * with some minimal testing and may not be optimal for all
-         * hardware setups, but it should work ok for most. It's
-         * faster than 1K buffers and 1M buffers, so change at your
-         * own peril. Note that it's useful to make it fit in the L2
-         * cache, so larger isn't necessarily better.
-         */
-        buf_size = st.st_size > 128 << 10 ? 128 << 10 : st.st_size;
-        buf = malloc(buf_size);
-        if (!buf) {
-                logit(NSLOG_RUNTIME_ERROR,TRUE,"Error: Unable to malloc(%ld) bytes: %s\n", buf_size, strerror(errno));
-                close(source_fd);
-                return ERROR;
-        }
-
-
-
-	/* open destination file for writing */
-        if((dest_fd=open(dest,O_WRONLY|O_TRUNC|O_CREAT|O_APPEND,0644)) < 0){
-                logit(NSLOG_RUNTIME_ERROR,TRUE,"Error: Unable to open file '%s' for writing: %s\n",dest,strerror(errno));
-                close(source_fd);
-                free(buf);
-                return ERROR;
-        }
-
-        /* most of the times, this loop will be gone through once */
-        while (tot_written < st.st_size) {
-                int loop_wr = 0;
-
-                rd_result = read(source_fd, buf, buf_size);
-                if (rd_result < 0) {
-                        if (errno == EAGAIN || errno == EINTR)
-                                continue;
-                        logit(NSLOG_RUNTIME_ERROR,TRUE,"Error: my_fcopy() failed to read from '%s': %s\n", source, strerror(errno));
-                        break;
-                }
-                tot_read += rd_result;
-                while (loop_wr < rd_result) {
-                        wr_result = write(dest_fd, buf + loop_wr, rd_result - loop_wr);
- 
-                        if (wr_result < 0) {
-                                if (errno == EAGAIN || errno == EINTR)
-                                        continue;
-                                logit(NSLOG_RUNTIME_ERROR,TRUE,"Error: my_fcopy() failed to write to '%s': %s\n", dest, strerror(errno));
-                                break;
+        /* open destination file for writing */
+        if((dest_fd=open(dest,O_WRONLY|O_TRUNC|O_CREAT|O_APPEND,0644))>0){
+                /* open source file for reading */
+                if((source_fd=open(source,O_RDONLY,0644))>0){
+                        /* copy file contents */
+                        while((bytes_read=read(source_fd,buffer,sizeof(buffer)))>0)
+                                write(dest_fd,buffer,bytes_read);
+                        close(source_fd);
+                        close(dest_fd);
+                        }
+                /* error opening the source file */
+                else{
+                        close(dest_fd);
+                        logit(NSLOG_RUNTIME_ERROR,TRUE,"Error: Unable to open file '%s' for reading: %s\n",source,strerror(errno));
+                        return ERROR;
                         }
-                        loop_wr += wr_result;
                 }
-                if (wr_result < 0)
-                        break;
-                tot_written += loop_wr;
-        }
-
-
-        /* clean up irregardless of how things went */
-        close(source_fd);
-        close(dest_fd);
-        free(buf);
- 
-        if (rd_result < 0 || wr_result < 0) {
-                /* don't leave half-written files around */
-                unlink(dest);
+        /* error opening destination file */
+        else{
+                close(dest_fd);
+                logit(NSLOG_RUNTIME_ERROR,TRUE,"Error: Unable to open file '%s' for writing: %s\n",dest,strerror(errno));
 		return ERROR;
-	}
+                }
 
 	return OK;
-}
+        }
 
 
 /******************************************************************/





More information about the icinga-checkins mailing list