From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel.Thibault@drdc-rddc.gc.ca (Thibault, Daniel) Date: Fri, 27 Jan 2012 11:36:59 -0500 Subject: [lttng-dev] [PATCH] lttng-tools lttng commands add_context.c :Return values of the cmd_add_context() chain of functions In-Reply-To: <4F22C7E1.6090004@efficios.com> References: <3CA1A65C03C1A34C81BBC6272782E696808924@valcartierex01.valcartier.drdc-rddc.gc.ca> <4F22C7E1.6090004@efficios.com> Message-ID: <3CA1A65C03C1A34C81BBC6272782E69680894E@valcartierex01.valcartier.drdc-rddc.gc.ca> -----Message d'origine----- De?: David Goulet [mailto:dgoulet at efficios.com] Envoy??: 27 janvier 2012 10:51 > > + /* > > + * 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. As for claiming the "API is well documented", I obviously beg to differ. :-) I put that comment in more to justify my code edits than just to "comment the code", so leaving it out of the patch is fine. > > 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. The problem with that behaviour is that it is not reflected in the return code of add_context(). The original code would return CMD_SUCCESS or CMD_ERROR depending on which -t option is processed last (assuming we have a -t option that fails), something I'm very uncomfortable with. A possible solution would be to add a new result code, say "CMD_WARNINGS", and treat the failing lttng_add_context() calls as warnings rather than errors. Or maybe add an option "-w|--stoponwarn" so the user can decide what the behaviour should be. 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