[lttng-dev] [PATCH] lttng-tools bin/lttng/conf.c : Document and enforce return values; bin/lttng/commands/create.c : Likewise

* [lttng-dev] [PATCH] lttng-tools bin/lttng/conf.c : Document and enforce return values; bin/lttng/commands/create.c : Likewise
       [not found] <mailman.104.1327689688.1530.lttng-dev@lists.lttng.org>
@ 2012-01-27 19:33 ` Thibault, Daniel
  2012-01-30 21:22   ` David Goulet
  0 siblings, 1 reply; 2+ messages in thread
From: Thibault, Daniel @ 2012-01-27 19:33 UTC (permalink / raw)


bin/lttng/commands/create.c : Completing the return values overhaul, setting --help usage() to stdout
bin/lttng/conf.c:
* In config_read_session_name(), moved the malloc to the beginning, to avoid the fscanf() loop from trying to write to a NULL
* In config_add_session_name(), dealt with another snprintf() occurrence
* write_config() (used by config_add_session_name() and others) could conceivably corrupt the config file; do we need to implement a simple commit/rollback mechanism?
------------------------------
From 0b675c0c4cba0c9bbbd9770005d7f260ddbf2a45 Fri, 27 Jan 2012 14:25:54 -0500
From: Daniel U. Thibault <daniel.thibault@drdc-rddc.gc.ca>
Date: Fri, 27 Jan 2012 14:25:44 -0500
Subject: [PATCH] lttng-tools bin/lttng/conf.c : Document and enforce return values; bin/lttng/commands/create.c : Likewise

diff --git a/src/bin/lttng/commands/create.c b/src/bin/lttng/commands/create.c
index 94ff634..b9ed2a2 100644
--- a/src/bin/lttng/commands/create.c
+++ b/src/bin/lttng/commands/create.c
@@ -116,15 +116,14 @@
 
 	ret = lttng_create_session(session_name, traces_path);
 	if (ret < 0) {
+		ret = CMD_ERROR;
 		goto error;
 	}
 
 	/* Init lttng session config */
 	ret = config_init(session_name);
 	if (ret < 0) {
-		if (ret == -1) {
-			ret = CMD_ERROR;
-		}
+		ret = CMD_ERROR;
 		goto error;
 	}
 
@@ -161,11 +160,10 @@
 	while ((opt = poptGetNextOpt(pc)) != -1) {
 		switch (opt) {
 		case OPT_HELP:
-			usage(stderr);
+			usage(stdout);
 			goto end;
 		case OPT_LIST_OPTIONS:
 			list_cmd_options(stdout, long_options);
-			ret = CMD_SUCCESS;
 			goto end;
 		default:
 			usage(stderr);
diff --git a/src/bin/lttng/conf.c b/src/bin/lttng/conf.c
index cdd264a..1ccd80b 100644
--- a/src/bin/lttng/conf.c
+++ b/src/bin/lttng/conf.c
@@ -32,7 +32,8 @@
 /*
  *  config_get_file_path
  *
- *  Return the path with '/CONFIG_FILENAME' added to it.
+ *  Returns the path with '/CONFIG_FILENAME' added to it;
+ *  path will be NULL if an error occurs.
  */
 char *config_get_file_path(char *path)
 {
@@ -50,7 +51,8 @@
 /*
  *  open_config
  *
- *  Return an open FILE pointer to the config file.
+ *  Returns an open FILE pointer to the config file;
+ *  on error, NULL is returned.
  */
 static FILE *open_config(char *path, const char *mode)
 {
@@ -77,7 +79,9 @@
 /*
  *  create_config_file
  *
- *  Create the empty config file a the path.
+ *  Creates the empty config file at the path.
+ *  On success, returns 0;
+ *  on error, returns -1.
  */
 static int create_config_file(char *path)
 {
@@ -101,6 +105,8 @@
  *  write_config
  *
  *  Append data to the config file in file_path
+ *  On success, returns 0;
+ *  on error, returns -1.
  */
 static int write_config(char *file_path, size_t size, char *data)
 {
@@ -116,7 +122,7 @@
 
 	/* Write session name into config file */
 	len = fwrite(data, size, 1, fp);
-	if (len < 1) {
+	if (len != 1) {
 		ret = -1;
 	}
 	fclose(fp);
@@ -127,7 +133,7 @@
 /*
  *  config_get_default_path
  *
- *  Return the HOME directory path. Caller MUST NOT free(3) the return pointer.
+ *  Returns the HOME directory path. Caller MUST NOT free(3) the return pointer.
  */
 char *config_get_default_path(void)
 {
@@ -137,7 +143,7 @@
 /*
  *  config_destroy
  *
- *  Destroy directory config and file config.
+ *  Destroys directory config and file config.
  */
 void config_destroy(char *path)
 {
@@ -160,7 +166,9 @@
 /*
  *  config_read_session_name
  *
- *  Return sesson name from the config file.
+ *  Returns the session name from the config file.
+ *  The caller is responsible for freeing the returned string.
+ *  On error, NULL is returned.
  */
 char *config_read_session_name(char *path)
 {
@@ -168,6 +176,11 @@
 	FILE *fp;
 	char var[NAME_MAX], *session_name;
 
+	session_name = malloc(NAME_MAX);
+	if (session_name == NULL) {
+		ERR("Out of memory");
+		goto error;
+	}
 	fp = open_config(path, "r");
 	if (fp == NULL) {
 		ERR("Can't find valid lttng config %s/.lttngrc", path);
@@ -175,7 +188,6 @@
 		goto error;
 	}
 
-	session_name = malloc(NAME_MAX);
 	while (!feof(fp)) {
 		if ((ret = fscanf(fp, "%[^'=']=%s\n", var, session_name)) != 2) {
 			if (ret == -1) {
@@ -206,14 +218,21 @@
  *  config_add_session_name
  *
  *  Write session name option to the config file.
+ *  On success, returns 0;
+ *  on error, returns -1.
  */
 int config_add_session_name(char *path, char *name)
 {
 	int ret;
 	char session_name[NAME_MAX];
 
+	/*
+	 * With GNU C <  2.1, snprintf returns -1 if the target buffer is too small;
+	 * With GNU C >= 2.1, snprintf returns the required size (excluding closing null)
+	 */
 	ret = snprintf(session_name, NAME_MAX, "session=%s\n", name);
-	if (ret < 0) {
+	if ((ret < 0) || (ret >= NAME_MAX)) {
+		ret = -1;
 		goto error;
 	}
 	ret = write_config(path, ret, session_name);
@@ -225,6 +244,8 @@
  *  config_init
  *
  *  Init configuration directory and file.
+ *  On success, returns 0;
+ *  on error, returns -1.
  */
 int config_init(char *session_name)
 {
------------------------------

Daniel U. Thibault
R & D pour la d?fense Canada - Valcartier (RDDC Valcartier) / Defence R&D Canada - Valcartier (DRDC Valcartier)
Syst?me de syst?mes (SdS) / System of Systems (SoS)
Solutions informatiques et exp?rimentations (SIE) / Computing Solutions and Experimentations (CSE)
2459 Boul. Pie XI Nord
Qu?bec, QC? G3J 1X5
CANADA
Vox : (418) 844-4000 x4245
Fax : (418) 844-4538
NAC: 918V QSDJ
Gouvernement du Canada?/ Government of Canada
<http://www.valcartier.drdc-rddc.gc.ca/>



^ permalink raw reply	[flat|nested] 2+ messages in thread