From d93dc72e6dbaaa254eaf7ccf0b014d6af1bb45ea Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 4 Mar 2010 11:42:38 +0100 Subject: [PATCH 1/2] Random C++ification: to_real_path() and current_config --- src/common.cc | 30 ++++++++++------------------ src/common.h | 8 ++++---- src/conky.cc | 52 +++++++++++++++++++++++-------------------------- src/conky.h | 2 +- src/imlib2.cc | 4 +++- src/llua.cc | 9 ++++----- src/tailhead.cc | 38 +++++++++++++++++++----------------- src/weather.cc | 6 ++---- 8 files changed, 68 insertions(+), 81 deletions(-) diff --git a/src/common.cc b/src/common.cc index 71feee30..274ac92b 100644 --- a/src/common.cc +++ b/src/common.cc @@ -99,31 +99,23 @@ double get_time(void) return tv.tv_sec + (tv.tv_usec / 1000000.0); } -/* Converts '~/...' paths to '/home/blah/...' assumes that 'dest' is at least - * DEFAULT_TEXT_BUFFER_SIZE. It's similar to variable_substitute, except only - * cheques for $HOME and ~/ in path */ -void to_real_path(char *dest, const char *source) +/* Converts '~/...' paths to '/home/blah/...' + * It's similar to variable_substitute, except only cheques for $HOME and ~/ in path */ +std::string to_real_path(const std::string &source) { - char tmp[DEFAULT_TEXT_BUFFER_SIZE]; - if (sscanf(source, "~/%s", tmp) || sscanf(source, "$HOME/%s", tmp)) { - char *homedir = getenv("HOME"); - if (homedir) { - snprintf(dest, DEFAULT_TEXT_BUFFER_SIZE, "%s/%s", homedir, tmp); - } else { - NORM_ERR("$HOME environment variable doesn't exist"); - strncpy(dest, source, DEFAULT_TEXT_BUFFER_SIZE); - } - } else if (dest != source) { //see changelog 2009-06-29 if you doubt that this check is necessary - strncpy(dest, source, DEFAULT_TEXT_BUFFER_SIZE); - } + const char *homedir = getenv("HOME"); + if(source.find("~/") == 0) + return homedir + source.substr(1); + else if(source.find("$HOME/") == 0) + return homedir + source.substr(5); + else + return source; } int open_fifo(const char *file, int *reported) { - char path[DEFAULT_TEXT_BUFFER_SIZE]; int fd = 0; - to_real_path(path, file); fd = open(file, O_RDONLY | O_NONBLOCK); if (fd == -1) { @@ -141,10 +133,8 @@ int open_fifo(const char *file, int *reported) FILE *open_file(const char *file, int *reported) { - char path[DEFAULT_TEXT_BUFFER_SIZE]; FILE *fp = 0; - to_real_path(path, file); fp = fopen(file, "r"); if (!fp) { diff --git a/src/common.h b/src/common.h index c5313534..dbae80cb 100644 --- a/src/common.h +++ b/src/common.h @@ -28,6 +28,7 @@ #include #include +#include #include #include #include "text_object.h" @@ -60,10 +61,9 @@ struct process *get_first_process(void); void get_cpu_count(void); double get_time(void); -/* Converts '~/...' paths to '/home/blah/...' assumes that 'dest' is at least - * DEFAULT_TEXT_BUFFER_SIZE. It's similar to variable_substitute, except only - * cheques for $HOME and ~/ in path */ -void to_real_path(char *dest, const char *source); +/* Converts '~/...' paths to '/home/blah/...' + * It's similar to variable_substitute, except only cheques for $HOME and ~/ in path */ +std::string to_real_path(const std::string &source); FILE *open_file(const char *file, int *reported); int open_fifo(const char *file, int *reported); void variable_substitute(const char *s, char *dest, unsigned int n); diff --git a/src/conky.cc b/src/conky.cc index 15614fe2..726446a6 100644 --- a/src/conky.cc +++ b/src/conky.cc @@ -329,7 +329,7 @@ static char *disp = NULL; struct information info; /* path to config file */ -char *current_config; +std::string current_config; /* set to 1 if you want all text to be in uppercase */ static unsigned int stuff_in_uppercase; @@ -2278,12 +2278,14 @@ static void main_loop(void) break; } #ifdef HAVE_SYS_INOTIFY_H - if (!disable_auto_reload && inotify_fd != -1 && inotify_config_wd == -1 && current_config != 0) { + if (!disable_auto_reload && inotify_fd != -1 + && inotify_config_wd == -1 && !current_config.empty()) { inotify_config_wd = inotify_add_watch(inotify_fd, - current_config, + current_config.c_str(), IN_MODIFY); } - if (!disable_auto_reload && inotify_fd != -1 && inotify_config_wd != -1 && current_config != 0) { + if (!disable_auto_reload && inotify_fd != -1 + && inotify_config_wd != -1 && !current_config.empty()) { int len = 0, idx = 0; fd_set descriptors; struct timeval time_to_wait; @@ -2301,13 +2303,13 @@ static void main_loop(void) struct inotify_event *ev = (struct inotify_event *) &inotify_buff[idx]; if (ev->wd == inotify_config_wd && (ev->mask & IN_MODIFY || ev->mask & IN_IGNORED)) { /* current_config should be reloaded */ - NORM_ERR("'%s' modified, reloading...", current_config); + NORM_ERR("'%s' modified, reloading...", current_config.c_str()); reload_config(); if (ev->mask & IN_IGNORED) { /* for some reason we get IN_IGNORED here * sometimes, so we need to re-add the watch */ inotify_config_wd = inotify_add_watch(inotify_fd, - current_config, + current_config.c_str(), IN_MODIFY); } break; @@ -2351,10 +2353,8 @@ void initialisation(int argc, char** argv); /* reload the config file */ static void reload_config(void) { - char *current_config_copy = strdup(current_config); clean_up(NULL, NULL); sleep(1); /* slight pause */ - current_config = current_config_copy; initialisation(argc_copy, argv_copy); } @@ -2418,7 +2418,6 @@ void clean_up(void *memtofree1, void* memtofree2) free_and_zero(tmpstring2); free_and_zero(text_buffer); free_and_zero(global_text); - free_and_zero(current_config); #ifdef BUILD_PORT_MONITORS tcp_portmon_clear(); @@ -3950,40 +3949,39 @@ static const struct option longopts[] = { void set_current_config() { /* check if specified config file is valid */ - if (current_config) { + if (not current_config.empty()) { struct stat sb; - if (stat(current_config, &sb) || + if (stat(current_config.c_str(), &sb) || (!S_ISREG(sb.st_mode) && !S_ISLNK(sb.st_mode))) { - NORM_ERR("invalid configuration file '%s'\n", current_config); - free_and_zero(current_config); + NORM_ERR("invalid configuration file '%s'\n", current_config.c_str()); + current_config.clear(); } } /* load current_config, CONFIG_FILE or SYSTEM_CONFIG_FILE */ - if (!current_config) { + if (current_config.empty()) { /* load default config file */ - char buf[DEFAULT_TEXT_BUFFER_SIZE]; FILE *fp; /* Try to use personal config file first */ - to_real_path(buf, CONFIG_FILE); - if (buf[0] && (fp = fopen(buf, "r"))) { - current_config = strndup(buf, max_user_text); + std::string buf = to_real_path(CONFIG_FILE); + if (!buf.empty() && (fp = fopen(buf.c_str(), "r"))) { + current_config = buf; fclose(fp); } /* Try to use system config file if personal config not readable */ - if (!current_config && (fp = fopen(SYSTEM_CONFIG_FILE, "r"))) { - current_config = strndup(SYSTEM_CONFIG_FILE, max_user_text); + if (current_config.empty() && (fp = fopen(SYSTEM_CONFIG_FILE, "r"))) { + current_config = SYSTEM_CONFIG_FILE; fclose(fp); } /* No readable config found */ - if (!current_config) { + if (current_config.empty()) { #define NOCFGFILEFOUND "no readable personal or system-wide config file found" #ifdef BUILD_BUILTIN_CONFIG - current_config = strdup("==builtin=="); + current_config = "==builtin=="; NORM_ERR(NOCFGFILEFOUND ", using builtin default"); #else @@ -4016,8 +4014,8 @@ void initialisation(int argc, char **argv) { } if(for_scripts == false) { set_current_config(); - load_config_file(current_config); - currentconffile = conftree_add(currentconffile, current_config); + load_config_file(current_config.c_str()); + currentconffile = conftree_add(currentconffile, current_config.c_str()); } /* init specials array */ @@ -4121,7 +4119,7 @@ void initialisation(int argc, char **argv) { #ifdef BUILD_X11 /* load font */ if (output_methods & TO_X) { - load_config_file_x11(current_config); + load_config_file_x11(current_config.c_str()); } #endif /* BUILD_X11 */ @@ -4216,7 +4214,6 @@ int main(int argc, char **argv) argv_copy = argv; g_signal_pending = 0; max_user_text = MAX_USER_TEXT_DEFAULT; - current_config = 0; memset(&info, 0, sizeof(info)); free_templates(); clear_net_stats(); @@ -4250,8 +4247,7 @@ int main(int argc, char **argv) case 'V': print_version(); /* doesn't return */ case 'c': - free_and_zero(current_config); - current_config = strndup(optarg, max_user_text); + current_config = optarg; break; case 'q': if (!freopen("/dev/null", "w", stderr)) diff --git a/src/conky.h b/src/conky.h index 7e530d03..331775df 100644 --- a/src/conky.h +++ b/src/conky.h @@ -322,7 +322,7 @@ void human_readable(long long, char *, int); extern unsigned int max_user_text; /* path to config file */ -extern char *current_config; +extern std::string current_config; #ifdef BUILD_X11 #define TO_X 1 diff --git a/src/imlib2.cc b/src/imlib2.cc index e0bd5d4c..6598417f 100644 --- a/src/imlib2.cc +++ b/src/imlib2.cc @@ -129,7 +129,9 @@ void cimlib_add_image(const char *args) free(cur); return; } - to_real_path(cur->name, cur->name); + strncpy(cur->name, to_real_path(cur->name).c_str(), 1024); + cur->name[1023] = 0; + // // now we check for optional args tmp = strstr(args, "-p "); if (tmp) { diff --git a/src/llua.cc b/src/llua.cc index 6cab830d..edc030ba 100644 --- a/src/llua.cc +++ b/src/llua.cc @@ -120,7 +120,7 @@ void llua_init(void) lua_pushstring(lua_L, BUILD_ARCH); lua_setglobal(lua_L, "conky_build_arch"); - lua_pushstring(lua_L, current_config); + lua_pushstring(lua_L, current_config.c_str()); lua_setglobal(lua_L, "conky_config"); lua_pushcfunction(lua_L, &llua_conky_parse); @@ -141,18 +141,17 @@ void llua_init(void) void llua_load(const char *script) { int error; - char path[DEFAULT_TEXT_BUFFER_SIZE]; llua_init(); - to_real_path(path, script); - error = luaL_dofile(lua_L, path); + std::string path = to_real_path(script); + error = luaL_dofile(lua_L, path.c_str()); if (error) { NORM_ERR("llua_load: %s", lua_tostring(lua_L, -1)); lua_pop(lua_L, 1); #ifdef HAVE_SYS_INOTIFY_H } else if (!llua_block_notify && inotify_fd != -1) { - llua_append_notify(path); + llua_append_notify(path.c_str()); #endif /* HAVE_SYS_INOTIFY_H */ } } diff --git a/src/tailhead.cc b/src/tailhead.cc index be135315..2ac6af67 100644 --- a/src/tailhead.cc +++ b/src/tailhead.cc @@ -44,11 +44,18 @@ struct headtail { int wantedlines; - char *logfile; + std::string logfile; char *buffer; int current_use; int max_uses; int reported; + + headtail() + : wantedlines(0), buffer(NULL), current_use(0), max_uses(0), reported(0) + {} + + ~headtail() + { free(buffer); } }; static void tailstring(char *string, int endofstring, int wantedlines) { @@ -73,26 +80,21 @@ static void tailstring(char *string, int endofstring, int wantedlines) { void free_tailhead(struct text_object *obj) { struct headtail *ht = (struct headtail *)obj->data.opaque; - if (!ht) - return; - free_and_zero(ht->logfile); - free_and_zero(ht->buffer); - free_and_zero(obj->data.opaque); + obj->data.opaque = NULL; + delete ht; } void init_tailhead(const char* type, const char* arg, struct text_object *obj, void* free_at_crash) { unsigned int args; - struct headtail *ht; + struct headtail *ht = new headtail; - ht = (struct headtail *)malloc(sizeof(struct headtail)); - memset(ht, 0, sizeof(struct headtail)); - - ht->logfile = (char*)malloc(DEFAULT_TEXT_BUFFER_SIZE); - memset(ht->logfile, 0, DEFAULT_TEXT_BUFFER_SIZE); + std::unique_ptr tmp(new char[DEFAULT_TEXT_BUFFER_SIZE]); + memset(tmp.get(), 0, DEFAULT_TEXT_BUFFER_SIZE); ht->max_uses = DEFAULT_MAX_HEADTAIL_USES; - args = sscanf(arg, "%s %d %d", ht->logfile, &ht->wantedlines, &ht->max_uses); + // XXX: Buffer overflow ? + args = sscanf(arg, "%s %d %d", tmp.get(), &ht->wantedlines, &ht->max_uses); if (args < 2 || args > 3) { free_tailhead(obj); CRIT_ERR(obj, free_at_crash, "%s needs a file as 1st and a number of lines as 2nd argument", type); @@ -102,7 +104,7 @@ void init_tailhead(const char* type, const char* arg, struct text_object *obj, v CRIT_ERR(obj, free_at_crash, "invalid arg for %s, next_check must be larger than 0", type); } if (ht->wantedlines > 0 && ht->wantedlines <= MAX_HEADTAIL_LINES) { - to_real_path(ht->logfile, ht->logfile); + ht->logfile = to_real_path(tmp.get()); ht->buffer = NULL; ht->current_use = 0; } else { @@ -131,9 +133,9 @@ static void print_tailhead(const char* type, struct text_object *obj, char *p, i strcpy(p, ht->buffer); ht->current_use++; }else{ //otherwise find the needed data - if(stat(ht->logfile, &st) == 0) { + if(stat(ht->logfile.c_str(), &st) == 0) { if (S_ISFIFO(st.st_mode)) { - fd = open_fifo(ht->logfile, &ht->reported); + fd = open_fifo(ht->logfile.c_str(), &ht->reported); if(fd != -1) { if(strcmp(type, "head") == 0) { for(i = 0; linescounted < ht->wantedlines; i++) { @@ -153,7 +155,7 @@ static void print_tailhead(const char* type, struct text_object *obj, char *p, i } close(fd); } else { - fp = open_file(ht->logfile, &ht->reported); + fp = open_file(ht->logfile.c_str(), &ht->reported); if(fp != NULL) { if(strcmp(type, "head") == 0) { for(i = 0; i < ht->wantedlines; i++) { @@ -173,7 +175,7 @@ static void print_tailhead(const char* type, struct text_object *obj, char *p, i } ht->buffer = strdup(p); } else { - CRIT_ERR(NULL, NULL, "$%s can't find information about %s", type, ht->logfile); + CRIT_ERR(NULL, NULL, "$%s can't find information about %s", type, ht->logfile.c_str()); } } return; diff --git a/src/weather.cc b/src/weather.cc index 32bf4e48..c56bbc4f 100644 --- a/src/weather.cc +++ b/src/weather.cc @@ -882,10 +882,9 @@ void load_xoap_keys(void) FILE *fp; char *par = (char *) malloc(11 * sizeof(char)); char *key = (char *) malloc(17 * sizeof(char)); - char *xoap = (char *) malloc(64 * sizeof(char)); - to_real_path(xoap, XOAP_FILE); - fp = fopen(xoap, "r"); + std::string xoap = to_real_path(XOAP_FILE); + fp = fopen(xoap.c_str(), "r"); if (fp != NULL) { if (fscanf(fp, "%10s %16s", par, key) == 2) { xoap_cc = std::string("?cc=*&link=xoap&prod=xoap&par=") @@ -899,7 +898,6 @@ void load_xoap_keys(void) } free(par); free(key); - free(xoap); } void scan_weather_forecast_arg(struct text_object *obj, const char *arg, void *free_at_crash) From af02eab4b6849e33bce9446957aab6e8edf7e525 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 4 Mar 2010 11:46:30 +0100 Subject: [PATCH 2/2] This appears to plug a leak. --- src/common.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/common.cc b/src/common.cc index 274ac92b..6528d6a2 100644 --- a/src/common.cc +++ b/src/common.cc @@ -272,7 +272,7 @@ struct update_cb { static struct update_cb update_cb_head; -static void *run_update_callback(void *) __attribute__((noreturn)); +static void *run_update_callback(void *); static int threading_started = 0; @@ -363,13 +363,13 @@ static void *run_update_callback(void *data) { struct update_cb *ucb = static_cast(data); - if (!ucb || !ucb->func) pthread_exit(NULL); + if (!ucb || !ucb->func) return(NULL); while (1) { - if (sem_wait(&ucb->start_wait)) pthread_exit(NULL); - if (ucb->running == 0) pthread_exit(NULL); + if (sem_wait(&ucb->start_wait)) return(NULL); + if (ucb->running == 0) return(NULL); (*ucb->func)(); - if (sem_post(&ucb->end_wait)) pthread_exit(NULL); + if (sem_post(&ucb->end_wait)) return(NULL); } }