[lttng-dev] [PATCH] Enforce exactly one LTTNG_DOMAIN (add_context.c) (update 2) (REPLACES PREVIOUS PATCH)

David Goulet david.goulet at polymtl.ca
Mon Feb 13 10:37:28 EST 2012


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

Hi Daniel,

I've merge the memory leak part. For the domain enforcing part, I'll wait after
the 2.0 stable since we have much more to discuss about a new UNSPECIFIED domain
value and the use of 0 in the enum.

Thanks!
David

On 12-02-10 04:22 PM, Thibault, Daniel wrote:
> From 38de214f7fb42f814b421eb8c9b17f4fd7cfd343 Fri, 10 Feb 2012 16:19:34 -0500
> From: Daniel U. Thibault <daniel.thibault at drdc-rddc.gc.ca>
> Date: Fri, 10 Feb 2012 16:18:53 -0500
> Subject: [PATCH] Enforce exactly one LTTNG_DOMAIN (add_context.c) (update 2)
> 
> Also fix a memory leak.
> 
> Signed-off-by: Daniel U. Thibault <daniel.thibault at drdc-rddc.gc.ca>
> 
> diff --git a/src/bin/lttng/commands/add_context.c b/src/bin/lttng/commands/add_context.c
> index e499ef3..2192018 100644
> --- a/src/bin/lttng/commands/add_context.c
> +++ b/src/bin/lttng/commands/add_context.c
> @@ -37,6 +37,7 @@
>  static char *opt_session_name;
>  static int opt_kernel;
>  static int opt_userspace;
> +static enum lttng_domain_type opt_domain;
>  static char *opt_type;
>  #if 0
>  /* Not implemented yet */
> @@ -272,7 +273,7 @@
>  };
>  
>  /*
> - * Pretty print context type.
> + * Pretty print context type (FILE *ofp must be valid).
>   */
>  static void print_ctx_type(FILE *ofp)
>  {
> @@ -309,6 +310,7 @@
>  	fprintf(ofp, "will be added to all events and all channels.\n");
>  	fprintf(ofp, "Otherwise the context will be added only to the channel (-c)\n");
>  	fprintf(ofp, "and/or event (-e) indicated.\n");
> +	fprintf(ofp, "Exactly one domain (-k/--kernel or -u/--userspace) must be specified.\n");
>  	fprintf(ofp, "\n");
>  	fprintf(ofp, "Options:\n");
>  	fprintf(ofp, "  -h, --help               Show this help\n");
> @@ -325,9 +327,8 @@
>  #else
>  	fprintf(ofp, "  -u, --userspace          Apply to the user-space tracer\n");
>  #endif
> -	fprintf(ofp, "  -t, --type TYPE          Context type. You can repeat that option on\n");
> -	fprintf(ofp, "                           the command line to specify multiple contexts at once.\n");
> -	fprintf(ofp, "                           (--kernel preempts --userspace)\n");
> +	fprintf(ofp, "  -t, --type TYPE          Context type. You can repeat that option on the\n");
> +	fprintf(ofp, "                           command line to specify multiple contexts at once.\n");
>  	fprintf(ofp, "                           TYPE can be one of the strings below:\n");
>  	print_ctx_type(ofp);
>  	fprintf(ofp, "\n");
> @@ -340,7 +341,8 @@
>  }
>  
>  /*
> - * Find context numerical value from string.
> + * Find context numerical value from string (which must not be NULL).
> + * Returns the index on success, -1 on failure.
>   */
>  static int find_ctx_type_idx(const char *opt)
>  {
> @@ -360,6 +362,7 @@
>  
>  /*
>   * Add context to channel or event.
> + * Returns a CMD_* result value.
>   */
>  static int add_context(char *session_name)
>  {
> @@ -372,19 +375,12 @@
>  	memset(&context, 0, sizeof(context));
>  	memset(&dom, 0, sizeof(dom));
>  
> -	if (opt_kernel) {
> -		dom.type = LTTNG_DOMAIN_KERNEL;
> -	} else if (opt_userspace) {
> -		dom.type = LTTNG_DOMAIN_UST;
> -	} else {
> -		ERR("Please specify a tracer (-k/--kernel or -u/--userspace)");
> -		ret = CMD_ERROR;
> -		goto error;
> -	}
> +	/* cmd_add_context() ensures exactly one domain is set */
> +	dom.type = opt_domain;
>  
>  	handle = lttng_create_handle(session_name, &dom);
>  	if (handle == NULL) {
> -		ret = CMD_ERROR;
> +		ret = CMD_FATAL;
>  		goto error;
>  	}
>  
> @@ -431,23 +427,25 @@
>  		}
>  	}
>  
> -	ret = CMD_SUCCESS;
> +	/*
> +	 * CMD_WARNING means that at least one lttng_add_context() failed and
> +	 * tells the user to look on stderr for error(s).
> +	 */
> +	if (warn) {
> +		ret = CMD_WARNING;
> +	} else {
> +		ret = CMD_SUCCESS;
> +	}
>  
>  error:
>  	lttng_destroy_handle(handle);
>  
> -	/*
> -	 * This means that at least one add_context failed and tells the user to
> -	 * look on stderr for error(s).
> -	 */
> -	if (warn) {
> -		ret = CMD_WARNING;
> -	}
>  	return ret;
>  }
>  
>  /*
>   * Add context to channel or event.
> + * Returns a CMD_* result value.
>   */
>  int cmd_add_context(int argc, const char **argv)
>  {
> @@ -456,6 +454,7 @@
>  	struct ctx_type *type, *tmptype;
>  	char *session_name = NULL;
>  
> +	opt_domain = 0; /* LTTNG_DOMAIN_UNSPECIFIED */
>  	if (argc < 2) {
>  		usage(stderr);
>  		ret = CMD_ERROR;
> @@ -500,6 +499,13 @@
>  			}
>  			break;
>  		case OPT_USERSPACE:
> +			/* Early enforcement of requirement for exactly one domain */
> +			if (opt_domain != 0) { /* opt_domain != LTTNG_DOMAIN_UNSPECIFIED */
> +				ERR("More than one domain specified");
> +				ret = CMD_ERROR;
> +				goto end;
> +			}
> +			opt_domain = LTTNG_DOMAIN_UST;
>  			opt_userspace = 1;
>  #if 0
>  			opt_cmd_name = poptGetOptArg(pc);
> @@ -522,18 +528,34 @@
>  		goto end;
>  	}
>  
> -	if (!opt_session_name) {
> -		session_name = get_session_name();
> -		if (session_name == NULL) {
> -			ret = CMD_ERROR;
> -			goto end;
> -		}
> -	} else {
> -		session_name = opt_session_name;
> +	/* Enforce requirement for exactly one domain */
> +	/* Five LTTNG_DOMAIN_* values are currently planned */
> +	if (opt_kernel && opt_userspace) {
> +		ERR("More than one domain specified");
> +		ret = CMD_ERROR;
> +		goto end;
> +	} else if (!opt_kernel && !opt_userspace) {
> +		ERR("No domain specified: Use one of -k/--kernel or -u/--userspace");
> +		ret = CMD_ERROR;
> +		goto end;
> +	}
> +	/* OPT_KERNEL not in loop */
> +	if (opt_kernel) {
> +		opt_domain = LTTNG_DOMAIN_KERNEL;
> +	}
> +
> +	session_name = (opt_session_name ? opt_session_name : get_session_name() );
> +	if (session_name == NULL) {
> +		ret = CMD_ERROR;
> +		goto end;
>  	}
>  
>  	ret = add_context(session_name);
>  
> +	if (!opt_session_name) {
> +		free (session_name);
> +	}
> +
>  end:
>  	/* Cleanup allocated memory */
>  	cds_list_for_each_entry_safe(type, tmptype, &ctx_type_list.head, list) {
> ------------------------------
> 
>    Using static enum lttng_domain_type opt_domain and the implicit LTTNG_DOMAIN_UNSPECIFIED constant.
> 
> 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)

iQEcBAEBAgAGBQJPOS44AAoJEELoaioR9I02btIIAITizhErTUdWkSIagN2kHN2i
OVwfXfO6aPjLQOWnGaxUpDd8ecHgvZI1zc2vKPiioKHcBuU0EMuFuGllqd4mUFWx
kfirfBhbwVDuVODN4fAPtFsUivvjRC093zyEG/qbp/ErklE3/GAV0OvqyaJnPznG
ml4TBKlkOfDXPqBu5PqfflioZTH5iSjETzBaWZIc7ecWFM00cG5/kmIP1fjo0tb7
uvo7RSg3BS8ru8JViZqXNcxWjaJx6sdX5gRbzjZEwCZ4RmAgtKrdy+RgWNgk6KMb
2hEHY1gZiiNvbssFv7u/cJK5YE8n9/EUxpjwycEUM89RmQlZN+PBUqyiXgM9144=
=VkHT
-----END PGP SIGNATURE-----



More information about the lttng-dev mailing list