Date: Fri, 10 Feb 2012 10:28:06 -0500 From: Mathieu Desnoyers <mathieu.desnoy...@efficios.com> Subject: Re: [lttng-dev] [PATCH] Enforce exactly one LTTNG_DOMAIN (add_context.c) (updated) (REPLACES PREVIOUS PATCH)
> 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. That is precisely what brought about my LTTNG_DOMAIN_UNSPECIFIED suggestion. ------------------------------ Date: Fri, 10 Feb 2012 10:56:07 -0500 From: David Goulet <dgou...@efficios.com> Subject: Re: [lttng-dev] Adding LTTNG_DOMAIN_UNSPECIFIED > > I think it would be a good idea, in general, to keep the "0" values of > > enumerations as "unspecified" (or default) behavior, since as we extend > > the API structures, those being zero-padded, they will just have a > > semantic of "default" behavior with the zero value. > > Yes I agree, it's good idea to have the 0 being the default value. > > The zero value was removed for a reason some time ago. I can recall that there > was a good reason to that but I can't find the message since the commit > message > was really bad on that change (my bad...). > > For, now, I don't think it's a good idea to change it back and I see no reason > for doing so. I'll rather see the domain type handled in the command line tool > and not add an extra value to the API that we'll only be used on the client > side. > > Now, to put back the 0 in, we'll have to review *carefully* the code path > where > those values are used because I remember that it was triggering something > undesired (maybe removed at this time..). I don't see that for now as a bug > fix > so we'll keep that for the 2.1+ version. > > I do agree that a chain of if...else is not the "optimal" but considering > we'll > add more domain in later version, a check is necessary to ensure that -k and > -u > for example are not used together and the user is informed. > > FYI, Daniel, we have now deployed a Redmine at bugs.lttng.org for any bug > report > and feature request. I think the feature request ticket should be broadcast to > lttng-dev@ in the future. My proposal was not about adding a "legitimate" LTTNG_DOMAIN_UNSPECIFIED domain (which would then wind its way through the code and possibly cause the issues you hint at), but rather about adding an explicit place holder in the LTTNG_DOMAIN_* enum. It makes for more legible code if one initialises "lttng_domain_type opt_domain = LTTNG_DOMAIN_UNSPECIFIED;" instead of "int opt_domain = 0;". The latter form also runs the (unlikely in this case) risk of colliding with a new LTTNG_DOMAIN_* value added somewhere down the line. Since C can't do any type-checking with respect to enums, and can't test whether an int is a member of an enum or not, adding LTTNG_DOMAIN_UNSPECIFIED and marking it (in lttng.h) as "not for internal consumption" should offer no harm. 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@lists.lttng.org http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
