On Mon, May 07, 2018 at 02:58:11PM -0700, Jaegeuk Kim wrote:
> On 05/07, Adam Borowski wrote:
> > One offender I own has:
> > STORAGE DEVICE  
> > 1404x05xe3x07QGENEx00%x00x00x00x00x00x00x00x00x00x00x00x00x170x04
> > 
> > I limited it to pure ASCII only, but you might want to allow high-bit bytes,
> > either validating UTF-8 or assuming that mojibake is not as bad as beep and
> > zeroes as in the above example.  Or, cutting off at the first control
> > character might be a good alternative too.
> 
> How about printing only valid characters here? In my virtualbox, this is
> showing garbage characters.

I dug more, and it turns out that both the old way and what I had proposed
are totally wrong.  This field is not a real (ie, zero-terminated) string,
and its length is 16 bytes rather than 64 as in current code.  I went
through all SD card readers and USB sticks I had at hand, and indeed the
value is always padded with spaces.  Stuff past byte 16 might be zeroes,
spaces or garbage -- for real (SATA) disks it's clean, for USB stuff it
tends to be junk.  There's a four-byte "rev" field that's sometimes
interesting, but even if we want to print it, it would require a separator.

So, here's another stab:

-- >8 --
Subject: libf2fs: read "disk model" from SCSI ioctl the same way kernel does

Ref: drivers/scsi/scsi_scan.c scsi_add_lun()

Ie, fixed-width 16 bytes, assumed to be filled with spaces -- NOT
null-terminated; comments suggest that in some cases this field can be
truncated and filled with nulls but printf is fine with that.

The old code did read up to 64 characters, which produced garbage for
at least some USB-attached card readers.  It also special-cased '`'
character which the kernel does not.

Signed-off-by: Adam Borowski <kilob...@angband.pl>
---
 lib/libf2fs.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/lib/libf2fs.c b/lib/libf2fs.c
index 5ef0214..38480c5 100644
--- a/lib/libf2fs.c
+++ b/lib/libf2fs.c
@@ -880,13 +880,8 @@ int get_device_info(int i)
                io_hdr.timeout = 1000;
 
                if (!ioctl(fd, SG_IO, &io_hdr)) {
-                       int i = 16;
-
-                       MSG(0, "Info: [%s] Disk Model: ",
-                                       dev->path);
-                       while (reply_buffer[i] != '`' && i < 80)
-                               printf("%c", reply_buffer[i++]);
-                       printf("\n");
+                       MSG(0, "Info: [%s] Disk Model: %.16s\n",
+                                       dev->path, reply_buffer+16);
                }
 #endif
        } else {
-- 
2.17.0

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to