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

Sagi Grimberg
2015-05-10T10:45:16Z

On 5/10/2015 1:29 PM, Bart Van Assche wrote:
> 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",
>> +};
>

Hey Bart,

> Array definitions should occur in a .c file instead of in a .h file to
> avoid duplication of the array.

OK, I'll move to verbs.c (and cma.c) thanks.

> Additionally, please consider to use
> designated initializers since that would make the correspondence between
> enum label and text string easier to verify.

Sure.

>
>> +#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.
>

I can do that, any specific reason why to use inline over macro here?

Sagi.