Re: [Libcdio-devel] W32 problems

[LRN]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 22.10.2012 4:38, Rocky Bernstein wrote:
> On Sat, Sep 15, 2012 at 12:45 AM, LRN wrote:
>> 
>> Here are some patches for W32 version of libcdio, my test
>> results and observations.
>> 
>> 0002-use-sleep-compatible-to.mingw32.patch - there's no sleep()
>> on W32, but MinGW provides usleep.
>> 
>> 0004-disable-realpath-test-on.mingw32.patch - cdio_realpath() 
>> falls back to a simple strdup() on W32, so there's no sense in 
>> testing it (and we have no symlink() anyway).
>> 
>> 0006-fix-differences-in-_img_private_t-definition.all.patch - 
>> stdbool REDEFINES (!) "bool" to be _Bool, not int. Therefore all 
>> files that define something with "bool" type have to include 
>> stdbool.h, otherwise different source files will have different 
>> sizes for some structures.
>> 
>> 0007-fix-struct-packing-on-latest.mingw32.patch - as of gcc-4.7.0
>>  -mms-bitfields is enabled by default. Problem is, when it is 
>> enabled, gcc struct member packing attributes have no effect. 
>> Pragma pack has to be used to pack structures. This patch makes 
>> sure that on W32 structures are packed (requires configuring with
>>  CFLAGS=-std=iso9899:1999).
>> 
>> The output of `make check -k' is attached. As you can see, my 
>> patches did not quite fix the packing issue, so you might want
>> to look into that.
>> 
>> Lots of mmc errors, DeviceIoControl() simply returns 0 and sets 
>> error to 87. No idea why.
>> 
>> The tests that fail to find files might be failing because i'm 
>> building OOTSD.
>> 
>> Note that i'm running W7, and thus have no ASPI driver (AFAIK 
>> there are no free ASPI drivers).
>> 
> 
> In compiling for the next release cygwin compilation was broken. 
> I've redone *configure *to explicitly look for Windows header
> files *ntddcdrm.h* *ddk/scsi.h *and so o.
> *lib/driver/MSWindows/win32_ioctl.c *now includes based on these
> header defines tested in configure rather than assume __MINGW64__
> works has or doesn't have some particular set of headers
> 
> In short, please check to see that things still work in your 
> environment(s). Thanks.
> 

Tried 0.90 release.
Compiles out of the box ok.
Testuite runs OK OOTSD, all tests pass, except for testisocd.
3 tests were skipped:

testgetdevices test skipped until drive recording testing issues resolved
SKIP: testgetdevices.exe

- -- Unable find or access a CD-ROM drive with an ISO-9660 filesystem.
SKIP: testisocd.exe

SKIP: check_cue.sh


mmc3 passed, but printed some warnings.


The crash is, i think, due to run_mmc_cmd_win32ioctl(). Again.
See, it has this stack-allocated
  SCSI_PASS_THROUGH_WITH_BUFFERS sptwb;
structure, its size is 592 (it has a 512-bytes buffer in it).

Then you do:
if (SCSI_MMC_DATA_WRITE == e_direction) memcpy(&sptwb.DataBuf, p_buf,
i_buf);
Which is alarming, since there is no guarantee (AFAICS) that i_buf
does not exceed 512.

Then you do:
  sptwb.Spt.DataTransferLength= i_buf;
even though sptwb.DataBuf is only 512 bytes, while i_buf can be longer.

Then you calculate length as:
  length = offsetof(SCSI_PASS_THROUGH_WITH_BUFFERS, DataBuf) +
    sptwb.Spt.DataTransferLength;

which assumes that sptwb is followed by a
DataTransferLength-bytes-long buffer, even though it's only 512 bytes
long.

And finally:

  b_success = DeviceIoControl(p_env->h_device_handle,
                              IOCTL_SCSI_PASS_THROUGH,
                              (void *)&sptwb,
                              sizeof(SCSI_PASS_THROUGH),
                              &sptwb,
                              length,
                              &dw_bytes_returned,
                              NULL);

which writes back "length" bytes into &sptwb, corrupting the stack.


Test log (without USE_PASSTHROUGH_DIRECT) is attached.


Tried rebuilding with CPPFLAGS="-DUSE_PASSTHROUGH_DIRECT=1".
testisocd did not crash, and passed, but instead mmc_read failed:
Error: Sense key should be 5, got 0
FAIL: mmc_read.exe



Also, suggestion:
in default_cdio_log_handler() use this code:

#include <windows.h>
DebugBreak ();
ExitProcess (3);

instead of
abort ();

The reason for this is that abort() in MS C Runtime library simply
prints this:

This application has requested the Runtime to terminate it in an
unusual way.
Please contact the application's support team for more information.

to stderr, then calls raise(SIGABRT), which causes the process to
terminate abnormally (exit code 3), without flushing, closing, or
doing any cleanups.

The problem in all that is that gdb is not able to catch SIGABRT on
W32. Thus if you are debugging a process, and the process calls
abort(), it will die right under you, never giving you a chance to see
or debug the place where problem happened.

Thus i prefer to call DebugBreak(), which throws and exception that
gdb (or any debugger) can catch. If there's no debugger attached,
exception will go unhandled, crashing the process normally.

ExitProcess() is there to ensure that the process won't continue after
DebugBreak () if debugger is present (or exception was handled somehow).

If you don't like the idea of always using DebugBreak() (don't know
why anyone wouldn't like it, probably a religious thing...), then call
IsDebuggerPresent() first, to determine whether the process is being
debugger, and only call DebugBreak() if it is. Otherwise call abort().
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJQ/CCvAAoJEOs4Jb6SI2Cw2JsH/R9Zby4zR0iUA1vxS4krpl0I
2n/QwaBNBhj6n0pUkwcxOUqToV/RvyE3WWwyUh+DPRCF/h7HfL8uaBbwJFJ97W42
Dsu0gIWIbfz3IZPc//IG3QCnwJY8E9w6MgM2sXynVjaFKTebZbesB+gaEKGj8XC6
+PQ01qfBBZ2Yd1XGl1HM3dqcdfZIfpD5cFgXVXwbwpVb8XQAuP5ONpZybcJC8560
/emLo1XhrPsRLlwkMjqg7+rzWJCQFOjhXNgkH+dvgXHul3it26jndgMqM0BncieI
DxRF1RN/zZ/jBpg8CjGZVCinHeWX86dRJHg43R2RrFGJ7SZFDwwL+eCrNN2U0WE=
=gfY8
-----END PGP SIGNATURE-----

check.log
Description: Text document