[lttng-dev] [GSOC14]Can I Fix Bug #709 in this way?

[Yue Liu]

Hi,
    I want to submit some patches before writing my GSOC14 Proposals.
    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.):
        1.No fclose(FILE*) after fopen(FILE*).
        2.May be the second DEFAULT_DAEMON_HOME_CONFIGPATH macro (config.c
line 230) is incorrectly written. It should be
DEFAULT_DAEMON_SYSTEM_CONFIGPATH.

    According the description of Bug #709, I wrote the patches of this bug
as follows:
    1.I used func "ini_parse()" instead of "ini_parse_file()",  ini_parse()
will close the file when it's finished.
    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.
    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.
    I don't know whether it is correct and I hope to get some suggestion
and guidance from you:

diff --git a/src/common/config/config.c b/src/common/config/config.c
index 04bd2fd..40ae52d 100644
--- a/src/common/config/config.c
+++ b/src/common/config/config.c
@@ -192,53 +192,35 @@ int config_get_section_entries(const char
*override_path, const char *section,
  config_entry_handler_cb handler, void *user_data)
 {
  int ret = 0;
- FILE *config_file = NULL;
  struct handler_filter_args filter = { section, handler, user_data };

- if (override_path) {
- config_file = fopen(override_path, "r");
- if (config_file) {
- DBG("Loaded daemon configuration file at %s",
- override_path);
- } else {
- ERR("Failed to open daemon configuration file at %s",
- override_path);
- ret = -ENOENT;
- goto end;
- }
- } else {
- char *path = utils_get_home_dir();
+    /* Try to open the system daemon configuration file */
+    if (access(DEFAULT_DAEMON_SYSTEM_CONFIGPATH, R_OK) == 0) {
+        ret = ini_parse(DEFAULT_DAEMON_SYSTEM_CONFIGPATH,
+                (ini_entry_handler) config_entry_handler_filter, (void *)
&filter);
+    }

- /* Try to open the user's daemon configuration file */
- if (path) {
- ret = asprintf(&path, DEFAULT_DAEMON_HOME_CONFIGPATH, path);
- if (ret < 0) {
- goto end;
- }
+    char *path = utils_get_home_dir();

- ret = 0;
- config_file = fopen(path, "r");
- if (config_file) {
- DBG("Loaded daemon configuration file at %s", path);
- }
+    /* Try to open the user's daemon configuration file */
+    if (path) {
+        ret = asprintf(&path, DEFAULT_DAEMON_HOME_CONFIGPATH, path);
+        if (ret < 0) {
+            goto end;
+        }

- free(path);
- }
+        if (access(path, R_OK) == 0) {
+            ret = ini_parse(path,
+                    (ini_entry_handler) config_entry_handler_filter, (void
*) &filter);
+        }
+        free(path);
+    }

- /* Try to open the system daemon configuration file */
- if (!config_file) {
- config_file = fopen(DEFAULT_DAEMON_HOME_CONFIGPATH, "r");
- }
+ if (override_path && access(override_path, R_OK) == 0) {
+        ret = ini_parse(override_path,
+                (ini_entry_handler) config_entry_handler_filter, (void *)
&filter);
  }

- if (!config_file) {
- DBG("No daemon configuration file found.");
- goto end;
- }
-
- ret = ini_parse_file(config_file,
- (ini_entry_handler) config_entry_handler_filter, (void *) &filter);
-
 end:
  return ret;
 }
diff --git a/src/common/config/ini.c b/src/common/config/ini.c
index 7462aeb..f56e3e1 100644
--- a/src/common/config/ini.c
+++ b/src/common/config/ini.c
@@ -35,6 +35,7 @@
 #include <ctype.h>
 #include <string.h>

+#include <common/error.h>
 #include "ini.h"

 #if !INI_USE_STACK
@@ -206,8 +207,10 @@ int ini_parse(const char* filename, ini_entry_handler
handler, void* user)

  file = fopen(filename, "r");
  if (!file) {
+        ERR("Failed to open daemon configuration file at %s", filename);
  return -1;
  }
+    DBG("Loaded daemon configuration file at %s", filename);

  error = ini_parse_file(file, handler, user);
  fclose(file);


Best Regards,
Yue Liu
----------------------------------------------------------
Tsinghua University NISL-Lab(FIT-1-213)
TEL: 156-5279-8191
Mail: liuyue0310 at gmail.com

<liuyue0310 at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lttng.org/pipermail/lttng-dev/attachments/20140316/ca3f9cdd/attachment-0001.html>