Re: [PATCH] lsscsi: Fix truncation of 128-bit wwn

[Gris Ge <fge@redhat.com>]

[-- 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 --]