Skip to content

mmap IO example mistake #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
GrigorenkoPV opened this issue Jul 24, 2024 · 4 comments
Open

mmap IO example mistake #53

GrigorenkoPV opened this issue Jul 24, 2024 · 4 comments

Comments

@GrigorenkoPV
Copy link

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 */
@mwilck
Copy link

mwilck commented Aug 5, 2024

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?

@GrigorenkoPV
Copy link
Author

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.

@GrigorenkoPV
Copy link
Author

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?

@mwilck
Copy link

mwilck commented Aug 8, 2024

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants