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

[Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>]

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html