Fixes to make version 0.8 available in Debian

[leitao]

There are current 2 issues on version 0.8 that blocks it to be integrated in Debian.

1. The package is not compiling in all architectures anymore
2. The package is using the legacy directory for bash completion

Both of them are fixed in this pull request.

[msalau]

Hi @leitao

I believe one should use platform independent format specifier, but not cast a variable to match the format specifier.

You propose to change
printf("Current temperature : %llu\n", stats->curr);
to
printf("Current temperature : %llu\n", (long long unsigned) stats->curr);
I think the line should be changed to
printf("Current temperature : %" PRIu64 "\n", stats->curr);

The PRIu64 macro exists in C since C99 and is defined in inttypes.h.
Reference: http://en.cppreference.com/w/c/types/integer

Regards,
Maksim.

[leitao]

Hello Maksim,
In fact, your proposed fix works on my Debian/ppc64el environment, but not on my personal computer environment, as showed:

debra ➜  nvme-cli git:(priu) ✗ git diff | cat
diff --git a/fabrics.c b/fabrics.c
index 869592c..2768ac1 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -27,6 +27,7 @@
 #include <stdint.h>
 #include <unistd.h>
 #include <sys/ioctl.h>
+#include <inttypes.h>
 #ifdef LIBUDEV_EXISTS
 #include <libudev.h>
 #endif
@@ -266,7 +267,7 @@ static void print_discovery_log(struct nvmf_disc_rsp_page_hdr *log, int numrec)
 {
    int i;

-   printf("Discovery Log Number of Records %d, Generation counter %lld\n",
+   printf("Discovery Log Number of Records %d, Generation counter %"PRIu64"\n",
        numrec, log->genctr);

    for (i = 0; i < numrec; i++) {

debra ➜  nvme-cli git:(priu) ✗ make
cc -D_GNU_SOURCE -std=gnu99 -O2 -g -Wall -Werror -DNVME_VERSION='"0.8.dirty"' -c fabrics.c
fabrics.c: In function ‘print_discovery_log’:
fabrics.c:271:3: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘__le64’ [-Werror=format=]
   numrec, log->genctr);
   ^
cc1: all warnings being treated as errors
make: *** [fabrics.o] Error 1

[msalau]

Hi Breno,

Since log->genctr is little-endian, but host can be either big- or little-endian, we should use the le64toh() macro to convert the value first.

printf("Discovery Log Number of Records %d, Generation counter %"PRIu64"\n",
        numrec, (uint64_t)le64toh(log->genctr));

Regards,
Maksim.

[keithbusch]

Yes, I think Maksim has the right pattern to fix this. Thanks for bringing this up.

[leitao]

Thanks Keith and Maksim,

I followed your suggestion and now this pull request is updated.
I also added a fix fixed in Ubuntu to re-enable nvme-cli 0.8 to compile on 32-bits version systems.

Thank you,
Breno

[keithbusch]

Thanks!

[msalau]

Hi @leitao and @keithbusch

The 'Fix cast to enable nvme to compile on 64-bits architecture' commit has a change

-   cmd.addr = (__u64)(void *)buf;
+   cmd.addr = (unsigned long)(void *)buf;

I believe C99 has a type for that purpose too:

7.18.1.4 Integer types capable of holding object pointers
The following type designates an unsigned integer type with the property that any valid
pointer to void can be converted to this type, then converted back to pointer to void,
and the result will compare equal to the original pointer:
uintptr_t

The type is optional, but is defined in GCC.

Best regards,
Maksim.