Stefan Hajnoczi writes: > On Thu, Jan 10, 2013 at 08:23:13PM +0100, Lluís Vilanova wrote: >> diff --git a/trace/control-internal.h b/trace/control-internal.h >> new file mode 100644 >> index 0000000..188253a >> --- /dev/null >> +++ b/trace/control-internal.h >> @@ -0,0 +1,67 @@ >> +/* >> + * Interface for configuring and controlling the state of tracing events. >> + * >> + * Copyright (C) 2011-2012 Lluís Vilanova <vilan...@ac.upc.edu> >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> + * See the COPYING file in the top-level directory. >> + */
> Please add an include guard. It is only meant to be included from "trace/control.h", and only for the sake of maintaining declarations separated from inlined definitions. >> +static inline bool trace_event_is_pattern(const char *str) >> +{ >> + assert(str != NULL); >> + >> + while (*str != '\0') { >> + if (*str == '*') { >> + return true; >> + } >> + str++; >> + } >> + return false; > Equivalent to: > return strchr(str, '*'); Right. >> +static bool glob(const char *pat, const char *ev) > Name collision with glob(3). Please choose a different name so readers > know we are not calling the POSIX function. Right, I'll use 'pattern_glob'. >> +/** >> + * trace_print_events: >> + * >> + * Print the state of all events. >> + * >> + * Warning: This function must be implemented by each tracing backend. >> + * >> + * TODO: Should this be moved to generic code? > This is generic code so the TODO can be removed? No, I meant the opposite. That maybe this should *not* be in the generic control interface, but I don't know where it should be moved to then, as it's used in the simple, default and stderr backends, as well as in the monitor. Thanks, Lluis -- "And it's much the same thing with knowledge, for whenever you learn something new, the whole world becomes that much richer." -- The Princess of Pure Reason, as told by Norton Juster in The Phantom Tollbooth