Strange behavior of sg_inq -HH option

[mwilck]

For some VPD pages, sg_inq -HH behaves oddly:

$ sg_inq -p bdc /dev/sda
VPD INQUIRY: Block device characteristics VPD page (SBC)
  Non-rotating medium (e.g. solid state)
  Product type: Not specified
...
$ sg_inq -H -p bdc /dev/sda
VPD INQUIRY: Block device characteristics VPD page (SBC)
 00     00 b1 00 3c 00 01 00 00  00 00 00 00 00 00 00 00    ...<............
 10     00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00    ................
 20     00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00    ................
 30     00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00    ................
$ sudo ./src/.libs/sg_inq -HH -p bdc /dev/sda
VPD INQUIRY: Block device characteristics VPD page (SBC)
  Non-rotating medium (e.g. solid state)
  Product type: Not specified
...
$ sg_inq -HHH -p bdc /dev/sda
00 b1 00 3c 00 01 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
$ sg_inq -HHHH -p bdc /dev/sda
00 b1 00 3c 00 01 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00

As you can see, -HH decodes the output as if no -H was given at all. This happens for a couple of VPD pages, e.g. lbpv, too. The code looks as if this behavior was intended, but it doesn't make much sense to me.

This seems to be related to the recent JSON work. Bisection shows that (for the bdc VPD page) the behavior changed in the following range of commits afteer r1.47 (inside this range, sg_inq fails because of an unresolved symbol error):

• 048bd12 sg_inq+sg_vpd: more JSON work (add SG_C_CPP_ZERO_INIT)
• 4ea97e6 rescan-scsi-bus: sgdevice26: do not traverse sg class if scsi_device isnot added
• 23beedb sg_inq+sg_vpd: more JSON work (tpc)
• 640f7a7 cleanup warning and C++20 building issues
• aac31a3 (HEAD) rescan-scsi-bus.sh: fix handling of '-I ' option ; sg_inq+sg_vpd: more JSON work

The later commit 7e7308a ("sg_inq+sg_vpd: more updates but not finished") caused sg_inq -H -bdc (only one -H) to falsely print decoded output, too. That was fixed later in 3c4e78e ("C99 cleanup; sg_logs: more tape JSON").

I am uncertain what the inteded behavior of -HH should be, but probably not what's shown above.

[doug-gilbert]

Thanks for bringing that to my attention. There has been a big code re-arrangement under both sg_inq and sg_vpd since both output more or less the same VPD pages. When I was looking at JSON output, I realized that duplicating the code for sg_inq and sg_vpd was pretty wasteful. My thinking with -H (especially used multiple times) has evolved over time, and I use it a lot for testing since it can now be parsed (with the --inhex=FN option). I'm in the middle of adding the "cappid" changes (see the newly released sbc5r04.pdf and spc6r07.pdf), so it will be a few days before (svn) revision 999 hits this mirror. Hopefully that revision will improve the 'sg_inq -HH' and 'sg_vpd -HH' handling. They should output VPD pages in hex, with a running address/index in the first column, and with no ASCII rendering to the right. Hmmm, that contradicts what 'man sg3_utils' says under the '--hex' option. I need to sort this out ....

[doug-gilbert]

Recently checked-in svn revision 999 and it is mirrored here. Lots of --hex option related changes. The broad thrust is to follow what 'man sg3_utils(8)' says about that option. When used once, 16 bytes (or less) are output on each line with a leading address or index at the start of each line (starting at 0). When used twice, an ASCII rendering of the data is appended to each line. When used more than twice, the output is meant to be parsable. That means no leading address/index nor ASCII rendering to the right, just the hex bytes. But there are corner cases such as 'sg_inq -p ai -HHH' which outputs 16 bit hexadecimal words (little endian) suitable as input for hdparm to accept as input and decode. Using '-HHHHH' or one less in many utilities then each VPD, log, etc page is prefixed by a blank line and then a comment line starting with '#' followed by the name of that page. Thereafter that page is output in hex. So that output is both easy to parse and can also be cut up with a text editor to remove pages that are not of interest. The simplifies handling something like: 'sg_vpd -p all -HHHHH <scsi_device_node>'.
As for checked in code failing to compile, that seems strange. Before I check-in code I run clang -analyze and g++ -std=c++23 on it. Once it is checked in, github runs the code on many architectures and reports any warnings or errors to me. Note that recently I removed ./configure and all the Makefile.in files and their supports. So now ./autogen.sh or ./bootstrap needs to be run to generate ./configure and friends. The downside of that change is the ./autogen.sh needs autoconf installed. The upside is that it removed over 49,000 line of code from my svn and git repositories.