Re: [PATCH 01/15] qedf: Enable basic FDMI information.

[Bart Van Assche <Bart.VanAssche@xxxxxxxxxxx>]

On Tue, 2017-05-23 at 06:19 -0700, Dupuis, Chad wrote:
> +		snprintf(fc_host_serial_number(lport->host),
> +		    FC_SERIAL_NUMBER_SIZE,
> +		    "%02X%02X%02X%02X%02X%02X%02X%02X",
> +		    buf[7], buf[6], buf[5], buf[4],
> +		    buf[3], buf[2], buf[1], buf[0]);
> +	} else
> +		snprintf(fc_host_serial_number(lport->host),
> +		    FC_SERIAL_NUMBER_SIZE, "Unknown");
> +
> +	snprintf(fc_host_manufacturer(lport->host),
> +	    FC_SERIAL_NUMBER_SIZE, "%s", "Cavium Inc.");

Hello Chad,

I think this code would be a lot easier to read and to verify if it would be
modified as follows:
* Instead of using the fc_host_<field>() macros, assign shost_to_fc_host(lport->host)
  to a variable and change fc_host_<field>() into ...-><field>.
* Instead of using the FC_*_SIZE macros, use sizeof(...-><field>).

Thanks,

Bart.