<div dir="ltr"><div><div>Hi,</div><div> I want to submit some patches before writing my GSOC14 Proposals.</div><div> For Bug #709, I have read the source code, and found there may be more bugs in the func config_get_section_entries()(In src/common/config/config.c line 191.):</div>
<div> 1.No fclose(FILE*) after fopen(FILE*).</div><div> 2.May be the second DEFAULT_DAEMON_HOME_CONFIGPATH macro (config.c line 230) is incorrectly written. It should be DEFAULT_DAEMON_SYSTEM_CONFIGPATH.</div>
<div> </div><div> According the description of Bug #709, I wrote the patches of this bug as follows:</div><div> 1.I used func "ini_parse()" instead of "ini_parse_file()", ini_parse() will close the file when it's finished.</div>
<div> 2.The order of the configuration files is 1) System Wide (/etc/lttng/lttng.conf); 2) Local ($HOME/.lttng/lttng.conf); 3) Command line argument. Simply use the ini_parese() func to get the config info, and write back to filter struct, and update. I hope it will work.</div>
<div> 3.I move the DBG() and ERR() to func "ini_parse()"(ini.c), for debug and error infos. Just because I don't know where to put it will be better. </div><div> I don't know whether it is correct and I hope to get some suggestion and guidance from you:</div>
<div><br></div><div>diff --git a/src/common/config/config.c b/src/common/config/config.c</div><div>index 04bd2fd..40ae52d 100644</div><div>--- a/src/common/config/config.c</div><div>+++ b/src/common/config/config.c</div><div>
@@ -192,53 +192,35 @@ int config_get_section_entries(const char *override_path, const char *section,</div><div> <span class="" style="white-space:pre"> </span>config_entry_handler_cb handler, void *user_data)</div><div> {</div>
<div> <span class="" style="white-space:pre"> </span>int ret = 0;</div><div>-<span class="" style="white-space:pre"> </span>FILE *config_file = NULL;</div><div> <span class="" style="white-space:pre"> </span>struct handler_filter_args filter = { section, handler, user_data };</div>
<div> </div><div>-<span class="" style="white-space:pre"> </span>if (override_path) {</div><div>-<span class="" style="white-space:pre"> </span>config_file = fopen(override_path, "r");</div><div>-<span class="" style="white-space:pre"> </span>if (config_file) {</div>
<div>-<span class="" style="white-space:pre"> </span>DBG("Loaded daemon configuration file at %s",</div><div>-<span class="" style="white-space:pre"> </span>override_path);</div><div>-<span class="" style="white-space:pre"> </span>} else {</div>
<div>-<span class="" style="white-space:pre"> </span>ERR("Failed to open daemon configuration file at %s",</div><div>-<span class="" style="white-space:pre"> </span>override_path);</div><div>-<span class="" style="white-space:pre"> </span>ret = -ENOENT;</div>
<div>-<span class="" style="white-space:pre"> </span>goto end;</div><div>-<span class="" style="white-space:pre"> </span>}</div><div>-<span class="" style="white-space:pre"> </span>} else {</div><div>-<span class="" style="white-space:pre"> </span>char *path = utils_get_home_dir();</div>
<div>+ /* Try to open the system daemon configuration file */</div><div>+ if (access(DEFAULT_DAEMON_SYSTEM_CONFIGPATH, R_OK) == 0) {</div><div>+ ret = ini_parse(DEFAULT_DAEMON_SYSTEM_CONFIGPATH,</div><div>+ (ini_entry_handler) config_entry_handler_filter, (void *) &filter);</div>
<div>+ }</div><div> </div><div>-<span class="" style="white-space:pre"> </span>/* Try to open the user's daemon configuration file */</div><div>-<span class="" style="white-space:pre"> </span>if (path) {</div><div>
-<span class="" style="white-space:pre"> </span>ret = asprintf(&path, DEFAULT_DAEMON_HOME_CONFIGPATH, path);</div><div>-<span class="" style="white-space:pre"> </span>if (ret < 0) {</div><div>-<span class="" style="white-space:pre"> </span>goto end;</div>
<div>-<span class="" style="white-space:pre"> </span>}</div><div>+ char *path = utils_get_home_dir();</div><div> </div><div>-<span class="" style="white-space:pre"> </span>ret = 0;</div><div>-<span class="" style="white-space:pre"> </span>config_file = fopen(path, "r");</div>
<div>-<span class="" style="white-space:pre"> </span>if (config_file) {</div><div>-<span class="" style="white-space:pre"> </span>DBG("Loaded daemon configuration file at %s", path);</div><div>-<span class="" style="white-space:pre"> </span>}</div>
<div>+ /* Try to open the user's daemon configuration file */</div><div>+ if (path) {</div><div>+ ret = asprintf(&path, DEFAULT_DAEMON_HOME_CONFIGPATH, path);</div><div>+ if (ret < 0) {</div>
<div>+ goto end;</div><div>+ }</div><div> </div><div>-<span class="" style="white-space:pre"> </span>free(path);</div><div>-<span class="" style="white-space:pre"> </span>}</div><div>+ if (access(path, R_OK) == 0) {</div>
<div>+ ret = ini_parse(path,</div><div>+ (ini_entry_handler) config_entry_handler_filter, (void *) &filter);</div><div>+ }</div><div>+ free(path);</div><div>+ }</div><div>
</div><div>-<span class="" style="white-space:pre"> </span>/* Try to open the system daemon configuration file */</div><div>-<span class="" style="white-space:pre"> </span>if (!config_file) {</div><div>-<span class="" style="white-space:pre"> </span>config_file = fopen(DEFAULT_DAEMON_HOME_CONFIGPATH, "r");</div>
<div>-<span class="" style="white-space:pre"> </span>}</div><div>+<span class="" style="white-space:pre"> </span>if (override_path && access(override_path, R_OK) == 0) {</div><div>+ ret = ini_parse(override_path,</div>
<div>+ (ini_entry_handler) config_entry_handler_filter, (void *) &filter);</div><div> <span class="" style="white-space:pre"> </span>}</div><div> </div><div>-<span class="" style="white-space:pre"> </span>if (!config_file) {</div>
<div>-<span class="" style="white-space:pre"> </span>DBG("No daemon configuration file found.");</div><div>-<span class="" style="white-space:pre"> </span>goto end;</div><div>-<span class="" style="white-space:pre"> </span>}</div>
<div>-</div><div>-<span class="" style="white-space:pre"> </span>ret = ini_parse_file(config_file,</div><div>-<span class="" style="white-space:pre"> </span>(ini_entry_handler) config_entry_handler_filter, (void *) &filter);</div>
<div>-</div><div> end:</div><div> <span class="" style="white-space:pre"> </span>return ret;</div><div> }</div><div>diff --git a/src/common/config/ini.c b/src/common/config/ini.c</div><div>index 7462aeb..f56e3e1 100644</div>
<div>--- a/src/common/config/ini.c</div><div>+++ b/src/common/config/ini.c</div><div>@@ -35,6 +35,7 @@</div><div> #include <ctype.h></div><div> #include <string.h></div><div> </div><div>+#include <common/error.h></div>
<div> #include "ini.h"</div><div> </div><div> #if !INI_USE_STACK</div><div>@@ -206,8 +207,10 @@ int ini_parse(const char* filename, ini_entry_handler handler, void* user)</div><div> </div><div> <span class="" style="white-space:pre"> </span>file = fopen(filename, "r");</div>
<div> <span class="" style="white-space:pre"> </span>if (!file) {</div><div>+ ERR("Failed to open daemon configuration file at %s", filename);</div><div> <span class="" style="white-space:pre"> </span>return -1;</div>
<div> <span class="" style="white-space:pre"> </span>}</div><div>+ DBG("Loaded daemon configuration file at %s", filename);</div><div> </div><div> <span class="" style="white-space:pre"> </span>error = ini_parse_file(file, handler, user);</div>
<div> <span class="" style="white-space:pre"> </span>fclose(file);</div><div><br></div></div><div><br></div>Best Regards,<br clear="all"><div><div dir="ltr"><div>Yue Liu</div><div>----------------------------------------------------------</div>
<div>Tsinghua University NISL-Lab(FIT-1-213)</div><div>TEL: 156-5279-8191</div><div>Mail: <a href="mailto:liuyue0310@gmail.com" target="_blank">liuyue0310@gmail.com</a></div><div><a href="mailto:liuyue0310@gmail.com" target="_blank"><br>
</a></div></div></div>
</div>