[lttng-dev] Adding LTTNG_DOMAIN_UNSPECIFIED

[Thibault, Daniel]

Date: Fri, 10 Feb 2012 10:28:06 -0500
From: Mathieu Desnoyers <mathieu.desnoyers at 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 <dgoulet at 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/>