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

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

[Chaitanya Kulkarni]

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 plugins/wdc/wdc-nvme.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/plugins/wdc/wdc-nvme.c b/plugins/wdc/wdc-nvme.c
index 9531e36..604a8a9 100644
--- a/plugins/wdc/wdc-nvme.c
+++ b/plugins/wdc/wdc-nvme.c
@@ -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
-- 
2.17.0

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

[Chaitanya Kulkarni]

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 plugins/wdc/wdc-nvme.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/plugins/wdc/wdc-nvme.c b/plugins/wdc/wdc-nvme.c
index 604a8a9..5ef965d 100644
--- a/plugins/wdc/wdc-nvme.c
+++ b/plugins/wdc/wdc-nvme.c
@@ -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;
-- 
2.17.0

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

[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(-)

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"
 #include <sys/ioctl.h>
 #define CREATE_CMD
 #include "wdc-nvme.h"
diff --git a/plugins/wdc/wdc-nvme.h b/plugins/wdc/wdc-nvme.h
index c2d892b..b7cbaa3 100644
--- a/plugins/wdc/wdc-nvme.h
+++ b/plugins/wdc/wdc-nvme.h
@@ -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"
-- 
2.17.0

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

[Keith Busch]

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?

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

[Chaitanya Kulkarni]

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

?On 10/26/18, 1:10 PM, "Keith Busch" <keith.busch@intel.com> 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?
    

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

[Keith Busch]

On Fri, Oct 26, 2018@08:11:37PM +0000, Chaitanya Kulkarni wrote:
> 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 "..." 

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

[Chaitanya Kulkarni]

From: Keith Busch <keith.busch@intel.com>
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
? 
 
On Fri, Oct 26, 2018@08:11:37PM +0000, Chaitanya Kulkarni wrote:
> 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.

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

[Chaitanya Kulkarni]

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
? 
 





From: Keith Busch <keith.busch@intel.com>
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
? 
?
On Fri, Oct 26, 2018@08:11:37PM +0000, Chaitanya Kulkarni wrote:
> 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.    

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

[Keith Busch]

On Mon, Oct 29, 2018@09:45:18PM +0000, Chaitanya Kulkarni wrote:
> Can we get patch 1 and 2 in ? maybe we can drop 3rd one.

Done.