From b4b5081b4303c7d1f1d6cbbfb080ff8e23dd7f91 Mon Sep 17 00:00:00 2001 From: Kern Sibbald Date: Wed, 2 Apr 2008 08:45:31 +0000 Subject: [PATCH] kes Modify run_program() and run_program_full_output() to use call by reference for the results string. This corrects a long standing problem where the address of the string may be changed in the subroutine but not in the calling program. git-svn-id: https://bacula.svn.sourceforge.net/svnroot/bacula/trunk@6720 91ce42f0-d328-0410-95d8-f526ca767f89 --- bacula/src/lib/bpipe.c | 66 ++++++++++++++------------------- bacula/src/lib/mem_pool.h | 1 + bacula/src/lib/protos.h | 4 +- bacula/src/stored/autochanger.c | 15 +++++--- bacula/src/stored/dev.c | 3 +- bacula/src/stored/dvd.c | 4 +- bacula/src/stored/stored.c | 6 ++- bacula/src/version.h | 4 +- bacula/technotes-2.3 | 13 +++++++ 9 files changed, 62 insertions(+), 54 deletions(-) diff --git a/bacula/src/lib/bpipe.c b/bacula/src/lib/bpipe.c index 58a22eb70f..449cc7f805 100644 --- a/bacula/src/lib/bpipe.c +++ b/bacula/src/lib/bpipe.c @@ -326,45 +326,41 @@ static void build_argc_argv(char *cmd, int *bargc, char *bargv[], int max_argv) * Returns: 0 on success * non-zero on error == berrno status */ -int run_program(char *prog, int wait, POOLMEM *results) +int run_program(char *prog, int wait, POOLMEM *&results) { BPIPE *bpipe; int stat1, stat2; char *mode; - mode = (char *)(results != NULL ? "r" : ""); + mode = (char *)"r"; bpipe = open_bpipe(prog, wait, mode); if (!bpipe) { return ENOENT; } - if (results) { - results[0] = 0; - int len = sizeof_pool_memory(results) - 1; - fgets(results, len, bpipe->rfd); - results[len] = 0; - if (feof(bpipe->rfd)) { - stat1 = 0; - } else { - stat1 = ferror(bpipe->rfd); - } - if (stat1 < 0) { - berrno be; - Dmsg2(150, "Run program fgets stat=%d ERR=%s\n", stat1, be.bstrerror(errno)); - } else if (stat1 != 0) { - Dmsg1(150, "Run program fgets stat=%d\n", stat1); - if (bpipe->timer_id) { - Dmsg1(150, "Run program fgets killed=%d\n", bpipe->timer_id->killed); - /* NB: I'm not sure it is really useful for run_program. Without the - * following lines run_program would not detect if the program was killed - * by the watchdog. */ - if (bpipe->timer_id->killed) { - stat1 = ETIME; - pm_strcat(results, _("Program killed by Bacula watchdog (timeout)\n")); - } + results[0] = 0; + int len = sizeof_pool_memory(results) - 1; + fgets(results, len, bpipe->rfd); + results[len] = 0; + if (feof(bpipe->rfd)) { + stat1 = 0; + } else { + stat1 = ferror(bpipe->rfd); + } + if (stat1 < 0) { + berrno be; + Dmsg2(150, "Run program fgets stat=%d ERR=%s\n", stat1, be.bstrerror(errno)); + } else if (stat1 != 0) { + Dmsg1(150, "Run program fgets stat=%d\n", stat1); + if (bpipe->timer_id) { + Dmsg1(150, "Run program fgets killed=%d\n", bpipe->timer_id->killed); + /* NB: I'm not sure it is really useful for run_program. Without the + * following lines run_program would not detect if the program was killed + * by the watchdog. */ + if (bpipe->timer_id->killed) { + stat1 = ETIME; + pm_strcpy(results, _("Program killed by Bacula (timeout)\n")); } } - } else { - stat1 = 0; } stat2 = close_bpipe(bpipe); stat1 = stat2 != 0 ? stat2 : stat1; @@ -388,7 +384,7 @@ int run_program(char *prog, int wait, POOLMEM *results) * non-zero on error == berrno status * */ -int run_program_full_output(char *prog, int wait, POOLMEM *results) +int run_program_full_output(char *prog, int wait, POOLMEM *&results) { BPIPE *bpipe; int stat1, stat2; @@ -397,21 +393,16 @@ int run_program_full_output(char *prog, int wait, POOLMEM *results) char *buf; const int bufsize = 32000; - if (results == NULL) { - return run_program(prog, wait, NULL); - } sm_check(__FILE__, __LINE__, false); tmp = get_pool_memory(PM_MESSAGE); buf = (char *)malloc(bufsize+1); + results[0] = 0; mode = (char *)"r"; bpipe = open_bpipe(prog, wait, mode); if (!bpipe) { - if (results) { - results[0] = 0; - } return ENOENT; } @@ -449,11 +440,10 @@ int run_program_full_output(char *prog, int wait, POOLMEM *results) */ if (bpipe->timer_id && bpipe->timer_id->killed) { Dmsg1(150, "Run program fgets killed=%d\n", bpipe->timer_id->killed); - pm_strcat(tmp, _("Program killed by Bacula watchdog (timeout)\n")); + pm_strcpy(tmp, _("Program killed by Bacula (timeout)\n")); stat1 = ETIME; } - int len = sizeof_pool_memory(results) - 1; - bstrncpy(results, tmp, len); + pm_strcpy(results, tmp); Dmsg3(1900, "resadr=0x%x reslen=%d res=%s\n", results, strlen(results), results); stat2 = close_bpipe(bpipe); stat1 = stat2 != 0 ? stat2 : stat1; diff --git a/bacula/src/lib/mem_pool.h b/bacula/src/lib/mem_pool.h index 0dd7231af1..cd8dcad6d0 100644 --- a/bacula/src/lib/mem_pool.h +++ b/bacula/src/lib/mem_pool.h @@ -91,6 +91,7 @@ public: POOL_MEM(int pool) { mem = get_pool_memory(pool); *mem = 0; } ~POOL_MEM() { free_pool_memory(mem); mem = NULL; } char *c_str() const { return mem; } + POOLMEM *&addr() { return mem; } int size() const { return sizeof_pool_memory(mem); } char *check_size(int32_t size) { mem = check_pool_memory_size(mem, size); diff --git a/bacula/src/lib/protos.h b/bacula/src/lib/protos.h index 8b5a1b908a..4197b31e74 100644 --- a/bacula/src/lib/protos.h +++ b/bacula/src/lib/protos.h @@ -318,8 +318,8 @@ char * encode_time (time_t time, char *buf); char * encode_mode (mode_t mode, char *buf); int do_shell_expansion (char *name, int name_len); void jobstatus_to_ascii (int JobStatus, char *msg, int maxlen); -int run_program (char *prog, int wait, POOLMEM *results); -int run_program_full_output (char *prog, int wait, POOLMEM *results); +int run_program (char *prog, int wait, POOLMEM *&results); +int run_program_full_output (char *prog, int wait, POOLMEM *&results); const char * job_type_to_str (int type); const char * job_status_to_str (int stat); const char * job_level_to_str (int level); diff --git a/bacula/src/stored/autochanger.c b/bacula/src/stored/autochanger.c index cd0b19a84c..07596604de 100644 --- a/bacula/src/stored/autochanger.c +++ b/bacula/src/stored/autochanger.c @@ -191,7 +191,7 @@ int autoload_device(DCR *dcr, int writing, BSOCK *dir) changer = edit_device_codes(dcr, changer, dcr->device->changer_command, "load"); dev->close(); Dmsg1(200, "Run program=%s\n", changer); - status = run_program_full_output(changer, timeout, results.c_str()); + status = run_program_full_output(changer, timeout, results.addr()); if (status == 0) { Jmsg(jcr, M_INFO, 0, _("3305 Autochanger \"load slot %d, drive %d\", status is OK.\n"), slot, drive); @@ -265,9 +265,8 @@ int get_autochanger_loaded_slot(DCR *dcr) Jmsg(jcr, M_INFO, 0, _("3301 Issuing autochanger \"loaded? drive %d\" command.\n"), drive); changer = edit_device_codes(dcr, changer, dcr->device->changer_command, "loaded"); - *results.c_str() = 0; Dmsg1(100, "Run program=%s\n", changer); - status = run_program_full_output(changer, timeout, results.c_str()); + status = run_program_full_output(changer, timeout, results.addr()); Dmsg3(100, "run_prog: %s stat=%d result=%s", changer, status, results.c_str()); if (status == 0) { loaded = str_to_int32(results.c_str()); @@ -333,6 +332,11 @@ bool unload_autochanger(DCR *dcr, int loaded) return false; } + /* Virtual disk autochanger */ + if (dcr->device->changer_command[0] == 0) { + return true; + } + if (loaded < 0) { loaded = get_autochanger_loaded_slot(dcr); } @@ -350,8 +354,7 @@ bool unload_autochanger(DCR *dcr, int loaded) dcr->device->changer_command, "unload"); dev->close(); Dmsg1(100, "Run program=%s\n", changer); - *results.c_str() = 0; - int stat = run_program_full_output(changer, timeout, results.c_str()); + int stat = run_program_full_output(changer, timeout, results.addr()); dcr->VolCatInfo.Slot = slot; if (stat != 0) { berrno be; @@ -449,7 +452,7 @@ static bool unload_other_drive(DCR *dcr, int slot) Dmsg2(200, "close dev=%s reserve=%d\n", dev->print_name(), dev->reserved_device); Dmsg1(100, "Run program=%s\n", changer_cmd); - int stat = run_program_full_output(changer_cmd, timeout, results.c_str()); + int stat = run_program_full_output(changer_cmd, timeout, results.addr()); dcr->VolCatInfo.Slot = save_slot; dcr->dev = save_dev; if (stat != 0) { diff --git a/bacula/src/stored/dev.c b/bacula/src/stored/dev.c index b2c772e515..381986c68c 100644 --- a/bacula/src/stored/dev.c +++ b/bacula/src/stored/dev.c @@ -1,7 +1,7 @@ /* Bacula® - The Network Backup Solution - Copyright (C) 2000-2007 Free Software Foundation Europe e.V. + Copyright (C) 2000-2008 Free Software Foundation Europe e.V. The main author of Bacula is Kern Sibbald, with contributions from many others, a complete list can be found in the file AUTHORS. @@ -2053,7 +2053,6 @@ bool DEVICE::do_mount(int mount, int dotimeout) timeout = 0; } results = get_memory(4000); - results[0] = 0; /* If busy retry each second */ Dmsg1(100, "do_mount run_prog=%s\n", ocmd.c_str()); diff --git a/bacula/src/stored/dvd.c b/bacula/src/stored/dvd.c index 142027f446..f798098bd0 100644 --- a/bacula/src/stored/dvd.c +++ b/bacula/src/stored/dvd.c @@ -1,7 +1,7 @@ /* Bacula® - The Network Backup Solution - Copyright (C) 2005-2007 Free Software Foundation Europe e.V. + Copyright (C) 2005-2008 Free Software Foundation Europe e.V. The main author of Bacula is Kern Sibbald, with contributions from many others, a complete list can be found in the file AUTHORS. @@ -254,7 +254,7 @@ bool dvd_write_part(DCR *dcr) } Dmsg2(20, "Write part: cmd=%s timeout=%d\n", ocmd.c_str(), timeout); - status = run_program_full_output(ocmd.c_str(), timeout, results.c_str()); + status = run_program_full_output(ocmd.c_str(), timeout, results.addr()); Dmsg2(20, "Write part status=%d result=%s\n", status, results.c_str()); dev->blank_dvd = false; diff --git a/bacula/src/stored/stored.c b/bacula/src/stored/stored.c index 94d466168d..e4a4d6eb49 100644 --- a/bacula/src/stored/stored.c +++ b/bacula/src/stored/stored.c @@ -1,7 +1,7 @@ /* Bacula® - The Network Backup Solution - Copyright (C) 2000-2007 Free Software Foundation Europe e.V. + Copyright (C) 2000-2008 Free Software Foundation Europe e.V. The main author of Bacula is Kern Sibbald, with contributions from many others, a complete list can be found in the file AUTHORS. @@ -445,6 +445,7 @@ static int check_resources() static void cleanup_old_files() { POOLMEM *cleanup = get_pool_memory(PM_MESSAGE); + POOLMEM *results = get_pool_memory(PM_MESSAGE); int len = strlen(me->working_directory); #if defined(HAVE_WIN32) pm_strcpy(cleanup, "del /q "); @@ -457,8 +458,9 @@ static void cleanup_old_files() } pm_strcat(cleanup, my_name); pm_strcat(cleanup, "*.spool"); - run_program(cleanup, 0, NULL); + run_program(cleanup, 0, results); free_pool_memory(cleanup); + free_pool_memory(results); } diff --git a/bacula/src/version.h b/bacula/src/version.h index 30f1147f92..b619bfc928 100644 --- a/bacula/src/version.h +++ b/bacula/src/version.h @@ -4,8 +4,8 @@ #undef VERSION #define VERSION "2.3.14" -#define BDATE "31 March 2008" -#define LSMDATE "31Mar08" +#define BDATE "01 April 2008" +#define LSMDATE "01Apr08" #define PROG_COPYRIGHT "Copyright (C) %d-2008 Free Software Foundation Europe e.V.\n" #define BYEAR "2008" /* year for copyright messages in progs */ diff --git a/bacula/technotes-2.3 b/bacula/technotes-2.3 index b7dd8ccac3..9a6b380cf6 100644 --- a/bacula/technotes-2.3 +++ b/bacula/technotes-2.3 @@ -24,6 +24,19 @@ Add long term statistics job table General: +02Apr08 +kes Modify run_program() and run_program_full_output() to use + call by reference for the results string. This corrects a long + standing problem where the address of the string may be changed + in the subroutine but not in the calling program. +01Apr08 +kes Re-enable code to remember last volume mounted on a non-tape + Autochanger. +kes Add patch supplied in bug #1068 that fixes a SD crash when using + a Virtual autochanger. +kes Generate correct JobMedia records during spooling/despooling when + running concurrent jobs. Thanks to Tom Ivar Helbekkmo + for excellent analysis and testing. 31Mar08 kes Tweak hash algorithm for htable using Martin Simmons idea for doing a circular shift. -- 2.39.5