From 4324865fb6275a224ce1112be2f7e593c15d9d58 Mon Sep 17 00:00:00 2001 From: Stefan Seyfried Date: Sun, 3 Mar 2013 19:18:13 +0100 Subject: [PATCH] helpers: improve my_system function Instead of hardcoding the maximum number of arguments to the my_system helper, pass a variable argument list. The function is deliberately source-incompatible with the old implementation (as opposed to a variant with a sentinel NULL argument, which would be compatible) to find all users and to make sure that new future users of this function are not overlooked during merges with other branches. Origin commit data ------------------ Branch: ni/coolstream Commit: https://github.com/neutrino-images/ni-neutrino/commit/96650cdd453992d101df32c89c3c8bf66faa4788 Author: Stefan Seyfried Date: 2013-03-03 (Sun, 03 Mar 2013) ------------------ This commit was generated by Migit --- src/driver/record.cpp | 2 +- src/gui/hdd_menu.cpp | 16 ++++++++-------- src/gui/moviebrowser.cpp | 2 +- src/gui/network_service.cpp | 4 ++-- src/gui/plugins.cpp | 2 +- src/gui/settings_manager.cpp | 4 ++-- src/gui/update.cpp | 2 +- src/neutrino.cpp | 6 +++--- src/system/configure_network.cpp | 4 ++-- src/system/helpers.cpp | 33 ++++++++++++++++++++++++++------ src/system/helpers.h | 3 +-- src/system/setting_helpers.cpp | 4 ++-- 12 files changed, 51 insertions(+), 31 deletions(-) diff --git a/src/driver/record.cpp b/src/driver/record.cpp index d988fdd8f..b1d4cb1c7 100644 --- a/src/driver/record.cpp +++ b/src/driver/record.cpp @@ -234,7 +234,7 @@ bool CRecordInstance::Stop(bool remove_event) if((autoshift && g_settings.auto_delete) /* || autoshift_delete*/) { snprintf(buf,sizeof(buf), "nice -n 20 rm -f \"%s.ts\" &", filename); - my_system("/bin/sh", "-c", buf); + my_system(3, "/bin/sh", "-c", buf); snprintf(buf,sizeof(buf), "%s.xml", filename); //autoshift_delete = false; unlink(buf); diff --git a/src/gui/hdd_menu.cpp b/src/gui/hdd_menu.cpp index bc96c17a1..a64f30fd0 100644 --- a/src/gui/hdd_menu.cpp +++ b/src/gui/hdd_menu.cpp @@ -298,10 +298,10 @@ int CHDDDestExec::exec(CMenuTarget* /*parent*/, const std::string&) if(hdparm_link){ //hdparm -M is not included in busybox hdparm! - my_system(hdparm, S_opt, opt); + my_system(3, hdparm, S_opt, opt); }else{ snprintf(M_opt, sizeof(M_opt),"-M%d", g_settings.hdd_noise); - my_system(hdparm, M_opt, S_opt, opt); + my_system(4, hdparm, M_opt, S_opt, opt); } free(namelist[i]); } @@ -362,7 +362,7 @@ static int umount_all(const char *dev) if (! access("/etc/mdev/mdev-mount.sh", X_OK)) { sprintf(buffer, "MDEV=%s%d ACTION=remove /etc/mdev/mdev-mount.sh block", d, i); printf("-> running '%s'\n", buffer); - my_system("/bin/sh", "-c", buffer); + my_system(3, "/bin/sh", "-c", buffer); } #endif sprintf(buffer, "/dev/%s%d", d, i); @@ -473,7 +473,7 @@ int CHDDFmtExec::exec(CMenuTarget* /*parent*/, const std::string& key) if(res != CMessageBox::mbrYes) return 0; - bool srun = my_system("killall", "-9", "smbd"); + bool srun = my_system(3, "killall", "-9", "smbd"); //res = check_and_umount(dst); //res = check_and_umount(src, dst); @@ -622,7 +622,7 @@ int CHDDFmtExec::exec(CMenuTarget* /*parent*/, const std::string& key) waitfordev(src, 30); /* mdev can somtimes takes long to create devices, especially after mkfs? */ printf("CHDDFmtExec: executing %s %s\n","/sbin/tune2fs -r 0 -c 0 -i 0", src); - my_system("/sbin/tune2fs", "-r 0", "-c 0", "-i 0", src); + my_system(5, "/sbin/tune2fs", "-r 0", "-c 0", "-i 0", src); _remount: unlink("/tmp/.nomdevmount"); @@ -711,7 +711,7 @@ _remount: #endif } _return: - if(!srun) my_system("smbd",NULL); + if (!srun) my_system(1, "smbd"); return menu_return::RETURN_REPAINT; } @@ -731,7 +731,7 @@ int CHDDChkExec::exec(CMenuTarget* /*parent*/, const std::string& key) printf("CHDDChkExec: key %s\n", key.c_str()); - bool srun = my_system("killall", "-9", "smbd"); + bool srun = my_system(3, "killall", "-9", "smbd"); //res = check_and_umount(dst); //res = check_and_umount(src, dst); @@ -820,6 +820,6 @@ ret1: } printf("CHDDChkExec: mount res %d\n", res); - if(!srun) my_system("smbd",NULL); + if (!srun) my_system(1, "smbd"); return menu_return::RETURN_REPAINT; } diff --git a/src/gui/moviebrowser.cpp b/src/gui/moviebrowser.cpp index 0aab3ced7..4ecc0cd81 100644 --- a/src/gui/moviebrowser.cpp +++ b/src/gui/moviebrowser.cpp @@ -3630,7 +3630,7 @@ int CDirMenu::exec(CMenuTarget* parent, const std::string & actionKey) if(dirState[number] == DIR_STATE_SERVER_DOWN) { printf("try to start server: %s %s\n","ether-wake", g_settings.network_nfs_mac[dirNfsMountNr[number]]); - if(my_system("ether-wake", g_settings.network_nfs_mac[dirNfsMountNr[number]]) != 0) + if (my_system(2, "ether-wake", g_settings.network_nfs_mac[dirNfsMountNr[number]]) != 0) perror("ether-wake failed"); dirOptionText[number]="STARTE SERVER"; diff --git a/src/gui/network_service.cpp b/src/gui/network_service.cpp index d0bf2e81a..aec0baaab 100644 --- a/src/gui/network_service.cpp +++ b/src/gui/network_service.cpp @@ -75,7 +75,7 @@ void CNetworkService::Start() { std::string cmd = command + " " + options; printf("CNetworkService::Start: %s %s\n", command.c_str(), options.c_str()); - my_system("/bin/sh", "-c", cmd.c_str()); + my_system(3, "/bin/sh", "-c", cmd.c_str()); enabled = true; TouchFile(); } @@ -84,7 +84,7 @@ void CNetworkService::Stop() { const char killall []= "killall"; printf("CNetworkService::Stop: %s %s\n", killall, command.c_str()); - my_system(killall, command.c_str()); + my_system(2, killall, command.c_str()); enabled = false; TouchFile(); } diff --git a/src/gui/plugins.cpp b/src/gui/plugins.cpp index a26c28d96..7e0b6637a 100644 --- a/src/gui/plugins.cpp +++ b/src/gui/plugins.cpp @@ -590,7 +590,7 @@ void CPlugins::startPlugin(int number,int /*param*/) frameBuffer->Lock(); //frameBuffer->setMode(720, 576, 8 * sizeof(fb_pixel_t)); printf("Starting %s\n", plugin_list[number].pluginfile.c_str()); - my_system(plugin_list[number].pluginfile.c_str(), NULL, NULL); + my_system(2, plugin_list[number].pluginfile.c_str(), NULL); //frameBuffer->setMode(720, 576, 8 * sizeof(fb_pixel_t)); frameBuffer->Unlock(); frameBuffer->paintBackground(); diff --git a/src/gui/settings_manager.cpp b/src/gui/settings_manager.cpp index 9bb4f3c60..c4aa9f735 100644 --- a/src/gui/settings_manager.cpp +++ b/src/gui/settings_manager.cpp @@ -108,7 +108,7 @@ int CSettingsManager::exec(CMenuTarget* parent, const std::string &actionKey) { const char backup_sh[] = "/bin/backup.sh"; printf("backup: executing [%s %s]\n",backup_sh, fileBrowser.getSelectedFile()->Name.c_str()); - my_system( backup_sh, fileBrowser.getSelectedFile()->Name.c_str() ); + my_system(2, backup_sh, fileBrowser.getSelectedFile()->Name.c_str()); } else ShowMsgUTF(LOCALE_MESSAGEBOX_ERROR, g_Locale->getText(LOCALE_SETTINGS_BACKUP_FAILED),CMessageBox::mbrBack, CMessageBox::mbBack, NEUTRINO_ICON_ERROR); @@ -126,7 +126,7 @@ int CSettingsManager::exec(CMenuTarget* parent, const std::string &actionKey) { const char restore_sh[] = "/bin/restore.sh"; printf("restore: executing [%s %s]\n", restore_sh, fileBrowser.getSelectedFile()->Name.c_str()); - my_system( restore_sh, fileBrowser.getSelectedFile()->Name.c_str() ); + my_system(2, restore_sh, fileBrowser.getSelectedFile()->Name.c_str()); } } return res; diff --git a/src/gui/update.cpp b/src/gui/update.cpp index d622f5e9c..590cc1a90 100644 --- a/src/gui/update.cpp +++ b/src/gui/update.cpp @@ -514,7 +514,7 @@ int CFlashUpdate::exec(CMenuTarget* parent, const std::string &actionKey) printf("[update] calling %s %s %s\n",install_sh, g_settings.update_dir, filename.c_str() ); #else printf("[update] calling %s %s %s\n",install_sh, g_settings.update_dir, filename.c_str() ); - my_system( install_sh, g_settings.update_dir, filename.c_str() ); + my_system(3, install_sh, g_settings.update_dir, filename.c_str()); #endif showGlobalStatus(100); ShowHintUTF(LOCALE_MESSAGEBOX_INFO, g_Locale->getText(LOCALE_FLASHUPDATE_READY)); // UTF-8 diff --git a/src/neutrino.cpp b/src/neutrino.cpp index c743e92c4..c7395fcd8 100644 --- a/src/neutrino.cpp +++ b/src/neutrino.cpp @@ -1946,8 +1946,8 @@ fprintf(stderr, "[neutrino start] %d -> %5ld ms\n", __LINE__, time_monotonic_ms #ifndef ASSUME_MDEV mkdir("/media/sda1", 0755); mkdir("/media/sdb1", 0755); - my_system("mount", "/dev/sda1", "/media/sda1"); - my_system("mount", "/dev/sdb1", "/media/sdb1"); + my_system(3, "mount", "/dev/sda1", "/media/sda1"); + my_system(3, "mount", "/dev/sdb1", "/media/sdb1"); #endif CFSMounter::automount(); @@ -2749,7 +2749,7 @@ _repeat: for(int i=0 ; i < NETWORK_NFS_NR_OF_ENTRIES ; i++) { if (strcmp(g_settings.network_nfs_local_dir[i],recordingDir) == 0) { printf("[neutrino] waking up %s (%s)\n",g_settings.network_nfs_ip[i].c_str(),recordingDir); - if(my_system("ether-wake",g_settings.network_nfs_mac[i]) != 0) + if (my_system(2, "ether-wake", g_settings.network_nfs_mac[i]) != 0) perror("ether-wake failed"); break; } diff --git a/src/system/configure_network.cpp b/src/system/configure_network.cpp index 30c347c5f..b62728c39 100644 --- a/src/system/configure_network.cpp +++ b/src/system/configure_network.cpp @@ -221,7 +221,7 @@ void CNetworkConfig::startNetwork(void) #ifdef DEBUG printf("CNetworkConfig::startNetwork: %s\n", cmd.c_str()); #endif - my_system("/bin/sh", "-c", cmd.c_str()); + my_system(3, "/bin/sh", "-c", cmd.c_str()); if (!inet_static) { init_vars(); @@ -235,7 +235,7 @@ void CNetworkConfig::stopNetwork(void) #ifdef DEBUG printf("CNetworkConfig::stopNetwork: %s\n", cmd.c_str()); #endif - my_system("/bin/sh", "-c", cmd.c_str()); + my_system(3, "/bin/sh", "-c", cmd.c_str()); } diff --git a/src/system/helpers.cpp b/src/system/helpers.cpp index cdf96ec09..9b348c317 100644 --- a/src/system/helpers.cpp +++ b/src/system/helpers.cpp @@ -37,7 +37,7 @@ #include #include #include -#include +#include #include #include @@ -70,12 +70,32 @@ int my_system(const char * cmd) if (!file_exists(cmd)) return -1; - return my_system(cmd, NULL); + return my_system(1, cmd); } -int my_system(const char * cmd, const char * arg1, const char * arg2, const char * arg3, const char * arg4, const char * arg5, const char * arg6) +int my_system(int argc, const char *arg, ...) { - int i=0 ,ret=0, childExit=0; + int i = 0, ret = 0, childExit = 0; +#define ARGV_MAX 64 + /* static right now but could be made dynamic if necessary */ + int argv_max = ARGV_MAX; + const char *argv[ARGV_MAX]; + va_list args; + argv[0] = arg; + va_start(args, arg); + + while(++i < argc) + { + if (i == argv_max) + { + fprintf(stderr, "my_system: too many arguments!\n"); + return -1; + } + argv[i] = va_arg(args, const char *); + } + argv[i] = NULL; /* sentinel */ + //fprintf(stderr,"%s:", __func__);for(i=0;argv[i];i++)fprintf(stderr," '%s'",argv[i]);fprintf(stderr,"\n"); + pid_t pid; int maxfd = getdtablesize();// sysconf(_SC_OPEN_MAX); switch (pid = vfork()) @@ -88,9 +108,9 @@ int my_system(const char * cmd, const char * arg1, const char * arg2, const char close(i); if (setsid() == -1) perror("my_system setsid"); - if(execlp(cmd, cmd, arg1, arg2, arg3, arg4, arg5, arg6, (char*)NULL)) + if (execvp(argv[0], (char * const *)argv)) { - std::string txt = "ERROR: my_system \"" + (std::string) cmd + "\""; + std::string txt = "ERROR: my_system \"" + (std::string) argv[0] + "\""; perror(txt.c_str()); ret = -1; } @@ -101,6 +121,7 @@ int my_system(const char * cmd, const char * arg1, const char * arg2, const char waitpid(pid, &childExit, 0); if(childExit != 0) ret = childExit; + va_end(args); return ret; } diff --git a/src/system/helpers.h b/src/system/helpers.h index 59d04a8bb..c87adb71f 100644 --- a/src/system/helpers.h +++ b/src/system/helpers.h @@ -22,9 +22,8 @@ Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ -int my_system(const char * cmd, const char * arg1, const char * arg2 = NULL, const char * arg3 = NULL, const char * arg4 = NULL, const char * arg5 = NULL, const char * arg6 = NULL); - int my_system(const char * cmd); +int my_system(int argc, const char *arg, ...); /* argc is number of arguments including command */ FILE* my_popen( pid_t& pid, const char *cmdstring, const char *type); int safe_mkdir(char * path); diff --git a/src/system/setting_helpers.cpp b/src/system/setting_helpers.cpp index 9a43b8d15..4385ff211 100644 --- a/src/system/setting_helpers.cpp +++ b/src/system/setting_helpers.cpp @@ -468,7 +468,7 @@ int CDataResetNotifier::exec(CMenuTarget* /*parent*/, const std::string& actionK return true; if(delete_all) { - my_system("/bin/sh", "-c", "rm -f /var/tuxbox/config/zapit/*.conf"); + my_system(3, "/bin/sh", "-c", "rm -f /var/tuxbox/config/zapit/*.conf"); CServiceManager::getInstance()->SatelliteList().clear(); CZapit::getInstance()->LoadSettings(); CZapit::getInstance()->GetConfig(zapitCfg); @@ -488,7 +488,7 @@ int CDataResetNotifier::exec(CMenuTarget* /*parent*/, const std::string& actionK CFrameBuffer::getInstance()->Clear(); } if(delete_chan) { - my_system("/bin/sh", "-c", "rm -f /var/tuxbox/config/zapit/*.xml"); + my_system(3, "/bin/sh", "-c", "rm -f /var/tuxbox/config/zapit/*.xml"); g_Zapit->reinitChannels(); } return ret;