On Wed 12 Dec 2012 11:54:10 PM PST, Bart Van Assche wrote: > On 12/13/12 00:22, Robert Love wrote: >> @@ -210,25 +210,23 @@ static ssize_t >> show_fcoe_fcf_device_##field(struct device *dev, \ >> #define fcoe_enum_name_search(title, table_type, table) \ >> static const char *get_fcoe_##title##_name(enum table_type >> table_key) \ >> { \ >> - int i; \ >> - char *name = NULL; \ >> - \ >> - for (i = 0; i < ARRAY_SIZE(table); i++) { \ >> - if (table[i].value == table_key) { \ >> - name = table[i].name; \ >> - break; \ >> - } \ >> - } \ >> - return name; \ >> + if (table_key >= ARRAY_SIZE(table)) \ >> + return NULL; \ >> + return table[table_key]; \ >> } > > The old code was safe if table_key < 0 but the new code not. Can > table_key be negative ? > >> +static char *fip_conn_type_names[] = { >> + [ FIP_CONN_TYPE_UNKNOWN ] = "Unknown", >> + [ FIP_CONN_TYPE_FABRIC ] = "Fabric", >> + [ FIP_CONN_TYPE_VN2VN ] = "VN2VN", >> +}; > > A minor nit: indentation style is inconsistent here. Two elements are > indented with a tab and another with eight spaces. > > Bart. Thanks for the review and comments, Bart. I agree with both of your points. I'll send an updated patch shortly. //Rob��.n��������+%������w��{.n�����{������ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f