RE: [PATCH v2 12/30] cxlflash: Refine host/device attributes

[David Laight]

From: Brian King
> Sent: 18 September 2015 22:35
...
> > +   for (i = 0; i < CXLFLASH_NUM_VLUNS; i++, buf += 22)
> 
> Rather than this bug prone hard coded 22, how about never incrementing buf 
> and do something
> similar to this:
> 
> > +   bytes += scnprintf(buf, PAGE_SIZE, "%03d: %016llX\n",
> > +  i, readq_be(_port[i]));
> 
>   bytes += scnprintf([bytes], PAGE_SIZE, "%03d: %016llX\n",
>  i, readq_be(_port[i]));
...

bytes += scnprintf(buf + bytes, PAGE_SIZE - bytes, 

You need to check scnprintf()'s return value though.
The above is wrong for libc's snprintf().

David

N�r��yb�X��ǧv�^�)޺{.n�+{���"�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj"��!�i

Re: [PATCH v2 12/30] cxlflash: Refine host/device attributes

[Matthew R. Ochs]

> On Sep 18, 2015, at 4:34 PM, Brian King  wrote:
> On 09/16/2015 04:29 PM, Matthew R. Ochs wrote:
>> 
>> +ssize_t bytes = 0;
>> +__be64 __iomem *fc_port;
>> +
>> +if (port >= NUM_FC_PORTS)
>> +return 0;
>> +
>> +fc_port = >afu_map->global.fc_port[port][0];
>> +
>> +for (i = 0; i < CXLFLASH_NUM_VLUNS; i++, buf += 22)
> 
> Rather than this bug prone hard coded 22, how about never incrementing buf 
> and do something
> similar to this:
> 
>> +bytes += scnprintf(buf, PAGE_SIZE, "%03d: %016llX\n",
>> +   i, readq_be(_port[i]));
> 
>   bytes += scnprintf([bytes], PAGE_SIZE, "%03d: %016llX\n",
>  i, readq_be(_port[i]));
> 
>> +return bytes;
>> +}
>> +

Great suggestion! Will fix in v3.


-matt
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 12/30] cxlflash: Refine host/device attributes

[Brian King]

On 09/16/2015 04:29 PM, Matthew R. Ochs wrote:
> Implement the following suggestions and add two new attributes
> to allow for debugging the port LUN table.
> 
>  - use scnprintf() instead of snprintf()
>  - use DEVICE_ATTR_RO and DEVICE_ATTR_RW
> 
> Signed-off-by: Matthew R. Ochs 
> Signed-off-by: Manoj N. Kumar 
> Suggested-by: Shane Seymour 
> ---
>  drivers/scsi/cxlflash/main.c | 180 
> +--
>  1 file changed, 138 insertions(+), 42 deletions(-)
> 

>  /**
> - * cxlflash_show_dev_mode() - presents the current mode of the device
> + * cxlflash_show_port_lun_table() - queries and presents the port LUN table
> + * @port:Desired port for status reporting.
> + * @afu: AFU owning the specified port.
> + * @buf: Buffer of length PAGE_SIZE to report back port status in ASCII.
> + *
> + * Return: The size of the ASCII string returned in @buf.
> + */
> +static ssize_t cxlflash_show_port_lun_table(u32 port,
> + struct afu *afu,
> + char *buf)
> +{
> + int i;
> + ssize_t bytes = 0;
> + __be64 __iomem *fc_port;
> +
> + if (port >= NUM_FC_PORTS)
> + return 0;
> +
> + fc_port = >afu_map->global.fc_port[port][0];
> +
> + for (i = 0; i < CXLFLASH_NUM_VLUNS; i++, buf += 22)

Rather than this bug prone hard coded 22, how about never incrementing buf and 
do something
similar to this:

> + bytes += scnprintf(buf, PAGE_SIZE, "%03d: %016llX\n",
> +i, readq_be(_port[i]));

bytes += scnprintf([bytes], PAGE_SIZE, "%03d: %016llX\n",
   i, readq_be(_port[i]));

> + return bytes;
> +}
> +



-- 
Brian King
Power Linux I/O
IBM Linux Technology Center

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html