From mboxrd@z Thu Jan 1 00:00:00 1970 From: compudj@krystal.dyndns.org (Mathieu Desnoyers) Date: Thu, 25 Nov 2010 11:00:30 -0500 Subject: [ltt-dev] [PATCH] Fix initial state bug In-Reply-To: <4CEE8348.3070800@gmail.com> References: <20101125102955.615258c7mapayccg@www.usherbrooke.ca> <4CEE8348.3070800@gmail.com> Message-ID: <20101125160030.GA10538@Krystal> * Yannick Brosseau (yannick.brosseau at gmail.com) wrote: > On 11/25/2010 10:29 AM, Francis Giraldeau wrote: >> This patch fixes a bug with the initial state in the control flow >> viewer. When >> a process was started before starting the trace, in some situation all >> the >> states of this process was wrong, because of a unknown initial state. > > +/* get a given quark from lttng module enum value */ > +#define enum_quark(e, f, names) (g_quark_from_string(names[ltt_event_get_unsigned(e, f)])) > + > /* Channel Quarks */ > > The enum_quark function name sounds too generic to me. Maybe ltt_event_enum_quark or ltt_enum_quark > > > +/* > + * Quark strings that matches lttng module enums found in statedump > + * Matching enums are from lttng-modules/ltt-statedump.c > + */ > +static const char *const lttng_thread_type_names[] = { > + "USER_THREAD", // LTTNG_USER_THREAD > + "KERNEL_THREAD" // LTTNG_KERNEL_THREAD > +}; > > Maybe these should be in their own header file to ease the maintenance > of the list ? Defining a static const in a header file is wrong. It should be declared in state.c. Also, shouldn't we declare enums in state.h to match those in ltt-statedump.c ? Also, we should change the occurences of e.g. "USER_THREAD" in state.c for: LTTV_STATE_USER_THREAD = g_quark_from_static_string(lttng_thread_type_names[ENUM_USER_THREAD]); and do the same for all these duplicated strings. Thanks, Mathieu > > > Yannick > > > _______________________________________________ > ltt-dev mailing list > ltt-dev at lists.casi.polymtl.ca > http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev > -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com