nvme-cli: fix array len to hold string of size 4

PATCH New Chaitanya Kulkarni 2 participants 9 messages Oct 26, 2018 – Oct 29, 2018 NVMe

lore

Share & tools

lore

Chaitanya Kulkarni

author

2018-10-26 17:08 UTC #0 lore raw

PATCH 1/3 nvme-cli: fix array len to hold string of size 4

Signed-off-by Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>

---
 plugins/wdc/wdc-nvme.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

plugins/wdc/wdc-nvme.c +4 -4

index 9531e36..604a8a9 100644
@@ -286,10 +286,10 @@ typedef enum _NVME_VU_DE_LOGPAGE_NAMES
 } NVME_VU_DE_LOGPAGE_NAMES;
 typedef struct _NVME_VU_DE_LOGPAGE_LIST
 {
-    NVME_VU_DE_LOGPAGE_NAMES logPageName;
-    __u32 logPageId;
-    __u32 logPageLen;
-    char  logPageIdStr[4];
+	NVME_VU_DE_LOGPAGE_NAMES logPageName;
+	__u32	logPageId;
+	__u32	logPageLen;
+	char	logPageIdStr[5];
 } NVME_VU_DE_LOGPAGE_LIST, *PNVME_VU_DE_LOGPAGE_LIST;
 
 typedef struct _WDC_NVME_DE_VU_LOGPAGES

Signature · 1 line

2.17.0

Chaitanya Kulkarni

author

2018-10-26 17:08 UTC #1 lore raw

PATCH 2/3 nvme-cli: fix endianness for the structure field

Signed-off-by Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>

---
 plugins/wdc/wdc-nvme.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

plugins/wdc/wdc-nvme.c +8 -7

index 604a8a9..5ef965d 100644
@@ -668,13 +668,14 @@ static int wdc_nvme_check_supported_log_page(int fd, __u8 log_id)
 
 	hdr_ptr = (struct wdc_c2_log_page_header *)data;
 
-	if (hdr_ptr->length > WDC_C2_LOG_BUF_LEN) {
-		fprintf(stderr, "ERROR : WDC : data length > buffer size : 0x%x\n", hdr_ptr->length);
+	if (le32_to_cpu(hdr_ptr->length) > WDC_C2_LOG_BUF_LEN) {
+		fprintf(stderr, "ERROR : WDC : data length > buffer size : 0x%x\n",
+				le32_to_cpu(hdr_ptr->length));
 		goto out;
 	}
 
 	ret = nvme_get_log(fd, 0xFFFFFFFF, WDC_NVME_GET_AVAILABLE_LOG_PAGES_OPCODE,
-			   false,  hdr_ptr->length, data);
+			   false,  le32_to_cpu(hdr_ptr->length), data);
 	/* parse the data until the List of log page ID's is found */
 	if (ret) {
 		fprintf(stderr, "ERROR : WDC : Unable to read C2 Log Page data, ret = %d\n", ret);

@@ -682,13 +683,13 @@ static int wdc_nvme_check_supported_log_page(int fd, __u8 log_id)
 	}
 
 	length = sizeof(struct wdc_c2_log_page_header);
-	while (length < hdr_ptr->length) {
+	while (length < le32_to_cpu(hdr_ptr->length)) {
 		sph = (struct wdc_c2_log_subpage_header *)(data + length);
 
-		if (sph->entry_id == WDC_C2_LOG_PAGES_SUPPORTED_ID) {
+		if (le32_to_cpu(sph->entry_id) == WDC_C2_LOG_PAGES_SUPPORTED_ID) {
 			cbs_data = (struct wdc_c2_cbs_data *)&sph->data;
 
-			for (i = 0; i < cbs_data->length; i++) {
+			for (i = 0; i < le32_to_cpu(cbs_data->length); i++) {
 				if (log_id == cbs_data->data[i]) {
 					found = 1;
 					ret = 0;

@@ -700,7 +701,7 @@ static int wdc_nvme_check_supported_log_page(int fd, __u8 log_id)
 				fprintf(stderr, "ERROR : WDC : Log Page 0x%x not supported\n", log_id);
 				fprintf(stderr, "WDC : Supported Log Pages:\n");
 				/* print the supported pages */
-				d((__u8 *)&sph->data + 4, sph->length - 12, 16, 1);
+				d((__u8 *)&sph->data + 4, le32_to_cpu(sph->length) - 12, 16, 1);
 				ret = -1;
 			}
 			break;

Signature · 1 line

2.17.0

Chaitanya Kulkarni

author

2018-10-26 17:08 UTC #2 lore raw

PATCH 3/3 nvme-cli: fix header paths for new structure

In reply to Chaitanya Kulkarni

Signed-off-by Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>

---
 plugins/wdc/wdc-nvme.c | 16 ++++++++--------
 plugins/wdc/wdc-nvme.h |  4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

plugins/wdc/wdc-nvme.c +8 -8

index 5ef965d..d9c879c 100644
@@ -31,14 +31,14 @@
 
 #include "linux/nvme_ioctl.h"
 
-#include "nvme.h"
-#include "nvme-print.h"
-#include "nvme-ioctl.h"
-#include "plugin.h"
-#include "json.h"
-
-#include "argconfig.h"
-#include "suffix.h"
+#include "../../nvme.h"
+#include "../../nvme-print.h"
+#include "../../nvme-ioctl.h"
+#include "../../plugin.h"
+#include "../../json.h"
+
+#include "../../argconfig.h"
+#include "../../suffix.h"
 #include <sys/ioctl.h>
 #define CREATE_CMD
 #include "wdc-nvme.h"

plugins/wdc/wdc-nvme.h +2 -2

index c2d892b..b7cbaa3 100644
@@ -4,7 +4,7 @@
 #if !defined(WDC_NVME) || defined(CMD_HEADER_MULTI_READ)
 #define WDC_NVME
 
-#include "cmd.h"
+#include "../../cmd.h"
 
 PLUGIN(NAME("wdc", "Western Digital vendor specific extensions"),
 	COMMAND_LIST(

@@ -24,4 +24,4 @@ PLUGIN(NAME("wdc", "Western Digital vendor specific extensions"),
 
 #endif
 
-#include "define_cmd.h"
+#include "../../define_cmd.h"

Signature · 1 line

2.17.0

Keith Busch

2018-10-26 20:07 UTC #3 lore raw

PATCH 3/3 nvme-cli: fix header paths for new structure

Quoted 20 lines · Chaitanya Kulkarni

Signed-off-by Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>

---
 plugins/wdc/wdc-nvme.c | 16 ++++++++--------
 plugins/wdc/wdc-nvme.h |  4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

plugins/wdc/wdc-nvme.c +0 -6

index 5ef965d..d9c879c 100644
@@ -31,14 +31,14 @@
 
 #include "linux/nvme_ioctl.h"
 
-#include "nvme.h"
-#include "nvme-print.h"
-#include "nvme-ioctl.h"
-#include "plugin.h"
-#include "json.h"
-

plugins/wdc/wdc-nvme.c line 41

Quoted 10 lines

plugins/wdc/wdc-nvme.c +8 -2

-#include "argconfig.h"
-#include "suffix.h"
+#include "../../nvme.h"
+#include "../../nvme-print.h"
+#include "../../nvme-ioctl.h"
+#include "../../plugin.h"
+#include "../../json.h"
+
+#include "../../argconfig.h"
+#include "../../suffix.h"

The Makefile has the include path set such that this shouldn't be necessary. Is the new directory structure causing a problem in any environments?

Chaitanya Kulkarni

author

2018-10-26 20:11 UTC #4 lore raw

PATCH 3/3 nvme-cli: fix header paths for new structure

No but, static analysis tool reported include path warnings, maybe we can ignore these warnings ?

?On 10/26/18, 1:10 PM, "Keith Busch" <[email protected]> wrote:

    On Fri, Oct 26, 2018@10:08:34AM -0700, Chaitanya Kulkarni wrote:
    > Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
    > ---
    >  plugins/wdc/wdc-nvme.c | 16 ++++++++--------
    >  plugins/wdc/wdc-nvme.h |  4 ++--
    >  2 files changed, 10 insertions(+), 10 deletions(-)
    > 
    > diff --git a/plugins/wdc/wdc-nvme.c b/plugins/wdc/wdc-nvme.c
    > index 5ef965d..d9c879c 100644
    > --- a/plugins/wdc/wdc-nvme.c
    > +++ b/plugins/wdc/wdc-nvme.c
    > @@ -31,14 +31,14 @@
    >  
    >  #include "linux/nvme_ioctl.h"
    >  
    > -#include "nvme.h"
    > -#include "nvme-print.h"
    > -#include "nvme-ioctl.h"
    > -#include "plugin.h"
    > -#include "json.h"
    > -
    > -#include "argconfig.h"
    > -#include "suffix.h"
    > +#include "../../nvme.h"
    > +#include "../../nvme-print.h"
    > +#include "../../nvme-ioctl.h"
    > +#include "../../plugin.h"
    > +#include "../../json.h"
    > +
    > +#include "../../argconfig.h"
    > +#include "../../suffix.h"
    
    The Makefile has the include path set such that this shouldn't be
    necessary. Is the new directory structure causing a problem in any
    environments?

Keith Busch

2018-10-26 20:17 UTC #5 lore raw

PATCH 3/3 nvme-cli: fix header paths for new structure

Quoted 1 line · Chaitanya Kulkarni

No but, static analysis tool reported include path warnings, maybe we can ignore these warnings ?

Oh, interesting! It ought to be safe to ignore, but I don't like that either. Does your analysis tool report issues if you replace the quoted includes with angle-brackets?

  #include <...>

instead of

  #include "..."

Chaitanya Kulkarni

author

2018-10-26 21:56 UTC #6 lore raw

PATCH 3/3 nvme-cli: fix header paths for new structure

From: Keith Busch <[email protected]>
Sent: Friday, October 26, 2018 1:17 PM
To: Chaitanya Kulkarni
Cc: linux-nvme at lists.infradead.org
Subject: Re: [PATCH 3/3] nvme-cli: fix header paths for new structure ?

Quoted 1 line · Chaitanya Kulkarni

No but, static analysis tool reported include path warnings, maybe we can ignore these warnings ?

Oh, interesting! It ought to be safe to ignore, but I don't like that either. Does your analysis tool report issues if you replace the quoted includes with angle-brackets?

? #include <...>

instead of

? #include "..."

It still reports warning, but adding include directory to the nvme-cli/ I was able to get rid of the warning.

Chaitanya Kulkarni

author

2018-10-29 21:45 UTC #7 lore raw

PATCH 3/3 nvme-cli: fix header paths for new structure

Can we get patch 1 and 2 in ? maybe we can drop 3rd one.

From: Chaitanya Kulkarni
Sent: Friday, October 26, 2018 2:56 PM
To: Keith Busch
Cc: linux-nvme at lists.infradead.org
Subject: Re: [PATCH 3/3] nvme-cli: fix header paths for new structure ?

Quoted 1 line · Chaitanya Kulkarni

No but, static analysis tool reported include path warnings, maybe we can ignore these warnings ?

Oh, interesting! It ought to be safe to ignore, but I don't like that either. Does your analysis tool report issues if you replace the quoted includes with angle-brackets?

? #include <...>

instead of

? #include "..."
???
It still reports warning, but adding include directory to the nvme-cli/? I was able to get rid of the warning.

Keith Busch

2018-10-29 22:02 UTC #8 lore raw

PATCH 3/3 nvme-cli: fix header paths for new structure

Quoted 1 line · Chaitanya Kulkarni

Can we get patch 1 and 2 in ? maybe we can drop 3rd one.

Done.