* [lttng-dev] [PATCH] lttng-tools bin/lttng/conf.c : Document and enforce return values; bin/lttng/commands/create.c : Likewise
2012-01-27 19:33 ` [lttng-dev] [PATCH] lttng-tools bin/lttng/conf.c : Document and enforce return values; bin/lttng/commands/create.c : Likewise Thibault, Daniel
@ 2012-01-30 21:22 ` David Goulet
0 siblings, 0 replies; 2+ messages in thread
From: David Goulet @ 2012-01-30 21:22 UTC (permalink / raw)
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Merged thanks!
David
On 12-01-27 02:33 PM, Thibault, Daniel wrote:
> 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/>
>
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
iQEcBAEBAgAGBQJPJworAAoJEELoaioR9I02vfIIAMGalhKGZiXvI2rBuZtwWDwI
uFDuWcSmFqBiaCYgo2gMU1wgidX1htaDs9m/1VVyYdLROZBONvWFz0MztuBzdsT9
K6y36xV7BiaMl57GSJCVhzw/9cjwKz5wp3uMlYGJGhOUwicIfzxyG0YoR3mBaBDo
9BFcG7zw3irBcswzV1hSvWLde5IFPuJmK/1+M3PFlsKsAgKF0/k0Ux5hYNJPB1o6
wNcWBvE+Fp/m4+umaoFoaDWDJ8AFlNAUZF9ypEw/mj/sRi+LS71JuhL8kE8a9bpu
4qUIyK9tRYC0hsdJu4S1FI6V1zZOX8iaP/vEu5GEaEnh5qzoO5td3X3vDF1C4I4=
=vyI9
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 2+ messages in thread