* Thibault, Daniel ([email protected]) wrote: > From 43aaf8dc6e9f38d31e66afb8e8f0c218fb32a07e Thu, 9 Feb 2012 17:36:21 -0500 > From: Daniel U. Thibault <[email protected]> > 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 <[email protected]> > > 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 > [email protected] > http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
