doug-gilbert/sg3_utils

mmap IO example mistake

Opened this issue · 4 comments

diff --git a/examples/sg_simple4.c b/examples/sg_simple4.c
index 95a5023b..6f3799ad 100644
--- a/examples/sg_simple4.c
+++ b/examples/sg_simple4.c
@@ -118,7 +118,7 @@ int main(int argc, char * argv[])
     io_hdr.mx_sb_len = sizeof(sense_buffer);
     io_hdr.dxfer_direction = SG_DXFER_FROM_DEV;
     io_hdr.dxfer_len = INQ_REPLY_LEN;
-    /* io_hdr.dxferp = inqBuff; // ignored in mmap-ed IO */
+    io_hdr.dxferp = inqBuff;
     io_hdr.cmdp = inq_cdb;
     io_hdr.sbp = sense_buffer;
     io_hdr.timeout = 20000;     /* 20000 millisecs == 20 seconds */

This looks correct, but some comments would be appreciated.
This code is ancient (2007).

Did you run the test program, and got a SEGV or similar error? Was the error gone after this change?

Without this change, ioctl returns -1 aka EFAULT (Bad address).

With this change, it works.

But since now it attempts to write to this address, you also have to mmap with | PROT_WRITE, which would explain why this comment is lying

/* since I know this program will only read from inqBuff then I use
PROT_READ rather than PROT_READ | PROT_WRITE */
inqBuff = (unsigned char *)mmap(NULL, 8000, PROT_READ | PROT_WRITE,

All of this stuff is weird.

You can also do this

diff --git a/examples/sg_simple4.c b/examples/sg_simple4.c
index 95a5023b..31c31c84 100644
--- a/examples/sg_simple4.c
+++ b/examples/sg_simple4.c
@@ -53,6 +53,7 @@ int main(int argc, char * argv[])
                                 {0x00, 0, 0, 0, 0, 0};
     unsigned char * inqBuff;
     unsigned char * inqBuff2;
+    unsigned char bufff[INQ_REPLY_LEN];
     sg_io_hdr_t io_hdr;
     char * file_name = 0;
     char ebuff[EBUFF_SZ];
@@ -118,7 +119,7 @@ int main(int argc, char * argv[])
     io_hdr.mx_sb_len = sizeof(sense_buffer);
     io_hdr.dxfer_direction = SG_DXFER_FROM_DEV;
     io_hdr.dxfer_len = INQ_REPLY_LEN;
-    /* io_hdr.dxferp = inqBuff; // ignored in mmap-ed IO */
+    io_hdr.dxferp = bufff;
     io_hdr.cmdp = inq_cdb;
     io_hdr.sbp = sense_buffer;
     io_hdr.timeout = 20000;     /* 20000 millisecs == 20 seconds */
@@ -148,7 +149,7 @@ int main(int argc, char * argv[])
     }
 
     if (ok) { /* output result if it is available */
-        char * p = (char *)inqBuff;
+        char * p = (char *)bufff;
         int f = (int)*(p + 7);
         printf("Some of the INQUIRY command's results:\n");
         printf("    %.8s  %.16s  %.4s  ", p + 8, p + 16, p + 32);

And it works too. Maybe kernel just ignore SG_FLAG_MMAP_IO?

I glanced at the kernel code, but couldn't figure it out at a quick glance. The flag not completely ignored, that's for sure. I've never seen any code using it so far.

@doug-gilbert, any idea?