Re: [PATCH 2/4] tracing/sched: Fix task states in sched switch event

From: Ingo Molnar
Date: Thu May 13 2010 - 09:36:00 EST



* Carsten Emde <C.Emde@xxxxxxxxx> wrote:

> Hi Ingo,
>
> > Hm, this is totally unreadable. What does 'TASK_STATE_X' mean??
> Would this be better?
> +#define MAKE_TASK_STATE_STRING(num) TASK_STATE_##num " (" DESCR_TASK_STATE_##num ")"
> static const char *task_state_array[] = {
> - "R (running)", /* 0 */
> - "S (sleeping)", /* 1 */
> - "D (disk sleep)", /* 2 */
> - "T (stopped)", /* 4 */
> - "t (tracing stop)", /* 8 */
> - "Z (zombie)", /* 16 */
> - "X (dead)", /* 32 */
> - "x (dead)", /* 64 */
> - "K (wakekill)", /* 128 */
> - "W (waking)", /* 256 */
> + MAKE_TASK_STATE_STRING(0),
> + MAKE_TASK_STATE_STRING(1),
> + MAKE_TASK_STATE_STRING(2),
> + MAKE_TASK_STATE_STRING(4),
> + MAKE_TASK_STATE_STRING(8),
> + MAKE_TASK_STATE_STRING(16),
> + MAKE_TASK_STATE_STRING(32),
> + MAKE_TASK_STATE_STRING(64),
> + MAKE_TASK_STATE_STRING(128),
> + MAKE_TASK_STATE_STRING(256)

The whole enumeration there is pointless in that .c file - it tells nothing to
the code reader.

If it cannot be expressed in a meaningful way then introduce
TASK_STATE_STRINGS_INIT construct that is defined next to the strings (in a .h
file or so) - that way it's a coherent whole.

Thanks,

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/