[lttng-dev] [PATCH] lttng-tools lttng commands add_context.c : Return values of the cmd_add_context() chain of functions

[David Goulet]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

So, I've merged a couple of patch here. The changed made to calibrate.c,
add_context.c and documentation added to liblttng-ctl.

Please, for typo/comments, feel free to submit only one patch. I've no problem
with that.

On 12-01-26 04:27 PM, Thibault, Daniel wrote:
> Document and ensure the return values of the cmd_add_context() chain of functions.
> Make add_context() stop looping over lttng_add_context() upon failure (otherwise it reports success or failure depending only on the last call's result).
> Also patches lib/lttng-ctl/lttng-ctl.c
> ------------------------------
> From 9de81398942fe0adeb5288483ecb70e2573e69f0 Thu, 26 Jan 2012 16:23:05 -0500
> From: Daniel U. Thibault <daniel.thibault at drdc-rddc.gc.ca>
> Date: Thu, 26 Jan 2012 16:22:54 -0500
> Subject: [PATCH] lttng-tools lttng commands add_context.c : Return values of the cmd_add_context() chain of functions
> 
> diff --git a/src/bin/lttng/commands/add_context.c b/src/bin/lttng/commands/add_context.c
> index d0f5b6f..1c63d90 100644
> --- a/src/bin/lttng/commands/add_context.c
> +++ b/src/bin/lttng/commands/add_context.c
> @@ -358,6 +358,7 @@
>  
>  /*
>   * Add context to channel or event.
> + * Returns one of the CMD_* result constants.
>   */
>  static int add_context(char *session_name)
>  {
> @@ -379,7 +380,7 @@
>  
>  	handle = lttng_create_handle(session_name, &dom);
>  	if (handle == NULL) {
> -		ret = -1;
> +		ret = CMD_ERROR;
>  		goto error;
>  	}
>  
> @@ -400,11 +401,17 @@
>  		}
>  		DBG("Adding context...");
>  
> +		/*
> +		 * lttng_add_context() returns the size
> +		 * of the returned payload data
> +		 * or a negative error code.
> +		 */

I did not had this because the API is well documented so it would duplicate
comments across the code and I prefer not since changing the API makes the
comments useless.

>  		ret = lttng_add_context(handle, &context, opt_event_name,
>  				opt_channel_name);
> +		/* Stop looping upon error */
>  		if (ret < 0) {
>  			fprintf(stderr, "%s: ", type->opt->symbol);
> -			continue;
> +			goto error;

Ok, here I am really not sure. The add-context command allows multiple "-t"
options and they are all added one by one and not in one single blob. So, if the
add context fails, you get an error on stderr and the next context is processed.
So, unless I missed something here, it's the correct behavior.

Btw, I've changed "fprintf(stderr" to ERR().

>  		} else {
>  			MSG("%s context %s added to %s event in %s",
>  					opt_kernel ? "kernel" : "UST", type->opt->symbol,
> @@ -412,7 +419,8 @@
>  					opt_channel_name ? opt_channel_name : "all channels");
>  		}
>  	}
> -
> +	/* Normalise return code upon success */
> +	ret = CMD_SUCCESS;

excellent.

I'll go over the last patches you sent soon.

Thanks!
David

>  error:
>  	lttng_destroy_handle(handle);
>  
> @@ -421,6 +429,7 @@
>  
>  /*
>   * Add context to channel or event.
> + * Returns one of the CMD_* result constants.
>   */
>  int cmd_add_context(int argc, const char **argv)
>  {
> @@ -445,8 +454,10 @@
>  			ret = CMD_SUCCESS;
>  			goto end;
>  		case OPT_TYPE:
> -			//Look up the index of opt_type in ctx_opts[] first,
> -			//so we don't have to free(type) on failure
> +			/*
> +			 * Look up the index of opt_type in ctx_opts[] first,
> +			 * so we don't have to free(type) on failure
> +			 */
>  			index = find_ctx_type_idx(opt_type);
>  			if (index < 0) {
>  				ERR("Unknown context type %s", opt_type);
> diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c
> index e7ebab5..ebb76ec 100644
> --- a/src/lib/lttng-ctl/lttng-ctl.c
> +++ b/src/lib/lttng-ctl/lttng-ctl.c
> @@ -462,6 +462,7 @@
>  
>  /*
>   * Add context to event or/and channel.
> + * Returns the size of the returned payload data or a negative error code.
>   */
>  int lttng_add_context(struct lttng_handle *handle,
>  		struct lttng_event_context *ctx, const char *event_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)

iQEcBAEBAgAGBQJPIsfgAAoJEELoaioR9I02krEIAKj5nPYXGLwzlPVmBYS5sdOX
jERXlUBFUNex79Db+kLK45Nas+LJ5p2J/peTX8qqIfK93Uxi8hnFDFmkd7/ffVV7
Kc41psvOO3BJqbDCmcGOIOMGcXMDkoGHGadX4ny4w6YkBEaB+TrkV0Ce6MTk97P7
b653VQho7H+BeChl3SIUneCjE3Hmcu2RjcMYGqCIcxrla4Eg07gTCnohzF6jO+oy
depTwrsR6/1giWNpIDagRzf6YfGi0ye/p+ZhkM1aPzN12HD7kgxksaiQDg5KHqtT
lnICZvlDZjCA0XeQsv/f1OkW0rukdKJuf9mve29UWhHriaMe1U7u+D36qnQkulo=
=mX0b
-----END PGP SIGNATURE-----