-----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 <[email protected]> > 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 <[email protected]> > > 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 > [email protected] > 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----- _______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
