Re: [PATCH RFC] IB/core, cma: Nice log-friendly string helpers

Bart Van Assche
2015-05-10T10:29:07Z

On 05/10/15 12:04, Sagi Grimberg wrote:
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index 65994a1..d615e63 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -412,6 +412,32 @@ enum ib_event_type {
> IB_EVENT_GID_CHANGE,
> };
>
> +static const char * const ib_events[] = {
> + "CQ_ERR",
> + "QP_FATAL",
> + "QP_REQ_ERR",
> + "QP_ACCESS_ERR",
> + "COMM_EST",
> + "SQ_DRAINED",
> + "PATH_MIG",
> + "PATH_MIG_ERR",
> + "DEVICE_FATAL",
> + "PORT_ACTIVE",
> + "PORT_ERR",
> + "LID_CHANGE",
> + "PKEY_CHANGE",
> + "SM_CHANGE",
> + "SRQ_ERR",
> + "SRQ_LIMIT_REACHED",
> + "QP_LAST_WQE_REACHED",
> + "CLIENT_REREGISTER",
> + "GID_CHANGE",
> +};

Array definitions should occur in a .c file instead of in a .h file to
avoid duplication of the array. Additionally, please consider to use
designated initializers since that would make the correspondence between
enum label and text string easier to verify.

> +#define IB_EVENT(event) \
> + ((event) < ARRAY_SIZE(ib_events) ? \
> + ib_events[(event)] : "UNRECOGNIZED_EVENT")
> +

Since a compiler is allowed to use a signed type to implement an enum,
please cast "event" to an unsigned type before comparing it with the
array size. Additionally, please consider to define IB_EVENT() as an
inline function instead of a preprocessor macro.

Thanks,

Bart.