[lttng-dev] [PATCH] Enforce exactly one LTTNG_DOMAIN (add_context.c) (updated) (REPLACES PREVIOUS PATCH)
Mathieu Desnoyers
mathieu.desnoyers at efficios.com
Fri Feb 10 10:32:02 EST 2012
* Thibault, Daniel (Daniel.Thibault at drdc-rddc.gc.ca) wrote:
> From 43aaf8dc6e9f38d31e66afb8e8f0c218fb32a07e Thu, 9 Feb 2012 17:36:21 -0500
> From: Daniel U. Thibault <daniel.thibault at drdc-rddc.gc.ca>
> Date: Thu, 9 Feb 2012 17:36:01 -0500
> Subject: [PATCH] Enforce exactly one LTTNG_DOMAIN (add_context.c) (updated)
>
> Also fix a memory leak.
The check for more than one domain should be done when parsing the -u /
-k options. They should set a current domain rather than setting a
opt_kernel/opt_userspace, so we can use a switch rather than a cascade
of if in the rest of the code.
Thanks,
Mathieu
>
> 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..dc8de4b 100644
> --- a/src/bin/lttng/commands/add_context.c
> +++ b/src/bin/lttng/commands/add_context.c
> @@ -272,7 +272,7 @@
> };
>
> /*
> - * Pretty print context type.
> + * Pretty print context type (FILE *ofp must be valid).
> */
> static void print_ctx_type(FILE *ofp)
> {
> @@ -309,6 +309,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 +326,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 +340,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 +361,7 @@
>
> /*
> * Add context to channel or event.
> + * Returns a CMD_* result value.
> */
> static int add_context(char *session_name)
> {
> @@ -372,19 +374,21 @@
> memset(&context, 0, sizeof(context));
> memset(&dom, 0, sizeof(dom));
>
> + /* cmd_add_context() ensures exactly one domain is set */
> 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;
> + /* TODO As LTTNG_DOMAIN_* values are added, capture them here */
> + /* This can be reached only if the TODO hasn't been done */
> + ret = CMD_FATAL;
> goto error;
> }
>
> handle = lttng_create_handle(session_name, &dom);
> if (handle == NULL) {
> - ret = CMD_ERROR;
> + ret = CMD_FATAL;
> goto error;
> }
>
> @@ -431,23 +435,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)
> {
> @@ -522,18 +528,30 @@
> 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;
> + }
> +
> + 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) {
> ------------------------------
>
> 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
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
More information about the lttng-dev
mailing list