* [PATCH] lsscsi: Fix truncation of 128-bit wwn
@ 2017-03-17 4:38 Vaibhav Jain
2017-03-17 10:28 ` Gris Ge
0 siblings, 1 reply; 3+ messages in thread
From: Vaibhav Jain @ 2017-03-17 4:38 UTC (permalink / raw)
To: Doug Gilbert
Cc: Vaibhav Jain, linux-scsi, Jon Grimm, Vipin K Parashar,
Ping Tian Han, Gris Ge
Currently with '--wwn' flag, 128-bit wwns gets truncated and their
last 3 hex-digits missing. Below is a comparison of wwn reported by
lsscsi compared to wwn info at /dev/disk/by-id directory.
% lsscsi -w 0:0:5:0
[0:0:5:0] disk 0x60050764008181941000000000000 /dev/sdad
% ls -l /dev/disk/by-id/wwn-*
lrwxrwxrwx. 1 root root 10 Oct 19 01:08 /dev/disk/by-id/wwn-0x600507640081819410000000000001b1 -> ../../sdad
To fix this, the patch increases the size of member wwn of struct
disk_wwn_node_entry to 35 chars to accommodate the extra '0x' prefix and
null terminator. Also the size of the buffer wwn_str thats used to
output wwn to the std-out is increased to match the corresponding
member of disk_wwn_node_entry.
Link: https://bugs.launchpad.net/ubuntu/+source/lsscsi/+bug/1636467
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1387263
Cc: Jon Grimm <jon.grimm@canonical.com>
Cc: Vipin K Parashar <vipin@linux.vnet.ibm.com>
Cc: Ping Tian Han <pthan@cn.ibm.com>
Cc: Gris Ge <fge@redhat.com>
Reported-by: Ping Tian Han <pthan@cn.ibm.com>
Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
Reviewed-by: Gris Ge <fge@redhat.com>
---
src/lsscsi.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/lsscsi.c b/src/lsscsi.c
index 974b3f1..f20adcf 100644
--- a/src/lsscsi.c
+++ b/src/lsscsi.c
@@ -210,8 +210,8 @@ struct dev_node_list {
static struct dev_node_list* dev_node_listhead = NULL;
struct disk_wwn_node_entry {
- char wwn[32];
- char disk_bname[12];
+ char wwn[35]; /* '0x' + wwn<128-bit> + <null-terminator> */
+ char disk_bname[12];
};
#define DISK_WWN_NODE_LIST_ENTRIES 16
@@ -2939,14 +2939,14 @@ one_sdev_entry(const char * dir_name, const char * devname,
}
if (wd[0]) {
char dev_node[LMAX_NAME] = "";
- char wwn_str[34];
+ char wwn_str[35];
enum dev_type typ;
typ = (FT_BLOCK == non_sg.ft) ? BLK_DEV : CHR_DEV;
if (get_wwn) {
if ((BLK_DEV == typ) &&
get_disk_wwn(wd, wwn_str, sizeof(wwn_str)))
- printf("%-30s ", wwn_str);
+ printf("%-34s ", wwn_str);
else
printf(" "
" ");
--
2.9.3
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] lsscsi: Fix truncation of 128-bit wwn
2017-03-17 4:38 [PATCH] lsscsi: Fix truncation of 128-bit wwn Vaibhav Jain
@ 2017-03-17 10:28 ` Gris Ge
2017-03-24 4:18 ` Vaibhav Jain
0 siblings, 1 reply; 3+ messages in thread
From: Gris Ge @ 2017-03-17 10:28 UTC (permalink / raw)
To: Vaibhav Jain
Cc: Doug Gilbert, linux-scsi, Jon Grimm, Vipin K Parashar,
Ping Tian Han
[-- Attachment #1: Type: text/plain, Size: 2156 bytes --]
On Fri, Mar 17, 2017 at 10:08:40AM +0530, Vaibhav Jain wrote:
> ---
> src/lsscsi.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/lsscsi.c b/src/lsscsi.c
> index 974b3f1..f20adcf 100644
> --- a/src/lsscsi.c
> +++ b/src/lsscsi.c
> @@ -210,8 +210,8 @@ struct dev_node_list {
> static struct dev_node_list* dev_node_listhead = NULL;
>
> struct disk_wwn_node_entry {
> - char wwn[32];
> - char disk_bname[12];
> + char wwn[35]; /* '0x' + wwn<128-bit> + <null-terminator> */
Please consider to define a constant instead of using a magic number
everywhere. Like:
#define _DISK_WWN_MAX_LEN 35
/* ^ WWN here is extracted from /dev/disk/by-id/wwn-<WWN> which is
* created by udev 60-persistent-storage.rules using ID_WWN_WITH_EXTENSION.
* The udev ID_WWN_WITH_EXTENSION is the combination of char wwn[17] and
* char wwn_vendor_extension[17] from struct scsi_id_device.
*/
> + char disk_bname[12];
Please use space instead of \t for indention.
> };
>
> #define DISK_WWN_NODE_LIST_ENTRIES 16
> @@ -2939,14 +2939,14 @@ one_sdev_entry(const char * dir_name, const char * devname,
> }
> if (wd[0]) {
> char dev_node[LMAX_NAME] = "";
> - char wwn_str[34];
> + char wwn_str[35];
Please use space instead of \t for indention.
> enum dev_type typ;
>
> typ = (FT_BLOCK == non_sg.ft) ? BLK_DEV : CHR_DEV;
> if (get_wwn) {
> if ((BLK_DEV == typ) &&
> get_disk_wwn(wd, wwn_str, sizeof(wwn_str)))
> - printf("%-30s ", wwn_str);
> + printf("%-34s ", wwn_str);
Please use constant instead of magic number:
printf("%-*s ", _DISK_WWN_MAX_LEN - 1, wwn_str);
> else
> printf(" "
> " ");
> --
> 2.9.3
>
--
Gris Ge
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] lsscsi: Fix truncation of 128-bit wwn
2017-03-17 10:28 ` Gris Ge
@ 2017-03-24 4:18 ` Vaibhav Jain
0 siblings, 0 replies; 3+ messages in thread
From: Vaibhav Jain @ 2017-03-24 4:18 UTC (permalink / raw)
To: Gris Ge
Cc: Doug Gilbert, linux-scsi, Jon Grimm, Vipin K Parashar,
Ping Tian Han
Hi Gris,
Thanks for reviewing the patch. I have posted a v2 patch addressing your
review comments. Can you please take a look.
Patch-Link:
https://www.mail-archive.com/linux-scsi@vger.kernel.org/msg59720.html
Gris Ge <fge@redhat.com> writes:
> On Fri, Mar 17, 2017 at 10:08:40AM +0530, Vaibhav Jain wrote:
>> ---
>> src/lsscsi.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/lsscsi.c b/src/lsscsi.c
>> index 974b3f1..f20adcf 100644
>> --- a/src/lsscsi.c
>> +++ b/src/lsscsi.c
>> @@ -210,8 +210,8 @@ struct dev_node_list {
>> static struct dev_node_list* dev_node_listhead = NULL;
>>
>> struct disk_wwn_node_entry {
>> - char wwn[32];
>> - char disk_bname[12];
>> + char wwn[35]; /* '0x' + wwn<128-bit> + <null-terminator> */
> Please consider to define a constant instead of using a magic number
> everywhere. Like:
> #define _DISK_WWN_MAX_LEN 35
> /* ^ WWN here is extracted from /dev/disk/by-id/wwn-<WWN> which is
> * created by udev 60-persistent-storage.rules using ID_WWN_WITH_EXTENSION.
> * The udev ID_WWN_WITH_EXTENSION is the combination of char wwn[17] and
> * char wwn_vendor_extension[17] from struct scsi_id_device.
> */
>> + char disk_bname[12];
> Please use space instead of \t for indention.
>> };
>>
>> #define DISK_WWN_NODE_LIST_ENTRIES 16
>> @@ -2939,14 +2939,14 @@ one_sdev_entry(const char * dir_name, const char * devname,
>> }
>> if (wd[0]) {
>> char dev_node[LMAX_NAME] = "";
>> - char wwn_str[34];
>> + char wwn_str[35];
> Please use space instead of \t for indention.
>> enum dev_type typ;
>>
>> typ = (FT_BLOCK == non_sg.ft) ? BLK_DEV : CHR_DEV;
>> if (get_wwn) {
>> if ((BLK_DEV == typ) &&
>> get_disk_wwn(wd, wwn_str, sizeof(wwn_str)))
>> - printf("%-30s ", wwn_str);
>> + printf("%-34s ", wwn_str);
> Please use constant instead of magic number:
> printf("%-*s ", _DISK_WWN_MAX_LEN - 1, wwn_str);
>> else
>> printf(" "
>> " ");
>> --
>> 2.9.3
>>
>
> --
> Gris Ge
--
Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
Linux Technology Center, IBM India Pvt. Ltd.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-03-24 4:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-17 4:38 [PATCH] lsscsi: Fix truncation of 128-bit wwn Vaibhav Jain
2017-03-17 10:28 ` Gris Ge
2017-03-24 4:18 ` Vaibhav Jain
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.