dlls/ntdll/cdrom.c breakage

[Gerald Pfeifer]

The following change to dlls/ntdll/cdrom.c

revision 1.57
  date: 2005/06/27 12:07:49;  author: julliard;  state: Exp;  lines: +13 -19
  Dmitry Timoshkov 
  Add a check for sg_io_hdr_t and (not tested) check for scsireq_t
  presence.

breaks on FreeBSD

cdrom.c: In function `CDROM_ScsiPassThroughDirect':
  cdrom.c:1419: error: invalid application of `sizeof' to an incomplete type
  cdrom.c:1411: warning: unused variable `io'
  cdrom.c: In function `CDROM_ScsiPassThrough':
  cdrom.c:1534: error: invalid application of `sizeof' to an incomplete type
  cdrom.c:1526: warning: unused variable `io'

which does not have struct request_sense, but does have struct 
scsi_request_sense in /usr/include/cam/scsi/scsi_all.h.

I would have tried to cobble up a patch guard these by #ifdef linux,
but I'm not sure whether the patch to cdrom.c really was correct as
it currently looks: defining struct linux_cdrom_generic_command in
cdrom.c seems like quite bad a hack, doesn't it?

Gerald

[Dmitry Timoshkov]

"Gerald Pfeifer"  wrote:
> The following change to dlls/ntdll/cdrom.c
>
> revision 1.57
>   date: 2005/06/27 12:07:49;  author: julliard;  state: Exp;  lines: +13 -19
>   Dmitry Timoshkov 
>   Add a check for sg_io_hdr_t and (not tested) check for scsireq_t
>   presence.
>
> breaks on FreeBSD
>
> cdrom.c: In function `CDROM_ScsiPassThroughDirect':
>   cdrom.c:1419: error: invalid application of `sizeof' to an incomplete type
>   cdrom.c:1411: warning: unused variable `io'
>   cdrom.c: In function `CDROM_ScsiPassThrough':
>   cdrom.c:1534: error: invalid application of `sizeof' to an incomplete type
>   cdrom.c:1526: warning: unused variable `io'
>
> which does not have struct request_sense, but does have struct 
> scsi_request_sense in /usr/include/cam/scsi/scsi_all.h.
>
> I would have tried to cobble up a patch guard these by #ifdef linux,
> but I'm not sure whether the patch to cdrom.c really was correct as
> it currently looks: defining struct linux_cdrom_generic_command in
> cdrom.c seems like quite bad a hack, doesn't it?

Attached patch is a workaround for the problem. A correct approach is to add a test
for presence of the 'struct request_sense' to configure and add appropriate #ifdef's
in the code.

Changelog:
    Dmitry Timoshkov 
    A workaround for missing struct request_sense on FreeBSD.

[Gerald Pfeifer]

On Wed, 29 Jun 2005, Dmitry Timoshkov wrote:
> > breaks on FreeBSD
> >
> > cdrom.c: In function `CDROM_ScsiPassThroughDirect':
> >   cdrom.c:1419: error: invalid application of `sizeof' to an incomplete type
> >   cdrom.c:1411: warning: unused variable `io'
> >   cdrom.c: In function `CDROM_ScsiPassThrough':
> >   cdrom.c:1534: error: invalid application of `sizeof' to an incomplete type
> >   cdrom.c:1526: warning: unused variable `io'
> > which does not have struct request_sense, but does have struct 
> > scsi_request_sense in /usr/include/cam/scsi/scsi_all.h.
>
> Attached patch is a workaround for the problem. A correct approach is to 
> add a test for presence of the 'struct request_sense' to configure and 
> add appropriate #ifdef's in the code.
>
> Changelog:
>     Dmitry Timoshkov 
>     A workaround for missing struct request_sense on FreeBSD.

Thanks for the quick response, Dmitry!  Unfortunately, the patch that
Alexandre now committed does not solve the problem for me, and my 
autoconf foo is a bit rusty:

cdrom.c: In function `CDROM_ScsiPassThroughDirect':
  drom.c:1423: error: invalid application of `sizeof' to an incomplete type
  cdrom.c: In function `CDROM_ScsiPassThrough':
  cdrom.c:1543: error: invalid application of `sizeof' to an incomplete type

Alexandre, please find below another patch which does not fix this either, 
but gets rid of some unused variable warnings.

Gerald

ChangeLog:
Avoid unused variable warnings in CDROM_ScsiPassThroughDir() and
CDROM_ScsiPassThrough().

Index: cdrom.c
===================================================================
RCS file: /home/wine/wine/dlls/ntdll/cdrom.c,v
retrieving revision 1.59
diff -u -3 -p -r1.59 cdrom.c
--- cdrom.c	29 Jun 2005 19:18:54 -0000	1.59
+++ cdrom.c	29 Jun 2005 21:36:26 -0000
@@ -1405,10 +1405,11 @@ static NTSTATUS CDROM_ScsiPassThroughDir
     int ret = STATUS_NOT_SUPPORTED;
 #ifdef HAVE_SG_IO_HDR_T_INTERFACE_ID
     sg_io_hdr_t cmd;
+    int io;
 #elif defined HAVE_SCSIREQ_T_CMD
     scsireq_t cmd;
-#endif
     int io;
+#endif

if (pPacket->Length < sizeof(SCSI_PASS_THROUGH_DIRECT))
         return STATUS_BUFFER_TOO_SMALL;
@@ -1524,10 +1525,11 @@ static NTSTATUS CDROM_ScsiPassThrough(in
     int ret = STATUS_NOT_SUPPORTED;
 #ifdef HAVE_SG_IO_HDR_T_INTERFACE_ID
     sg_io_hdr_t cmd;
+    int io;
 #elif defined HAVE_SCSIREQ_T_CMD
     scsireq_t cmd;
-#endif
     int io;
+#endif

if (pPacket->Length < sizeof(SCSI_PASS_THROUGH))
         return STATUS_BUFFER_TOO_SMALL;

[Dmitry Timoshkov]

"Gerald Pfeifer"  wrote:
> Thanks for the quick response, Dmitry!  Unfortunately, the patch that
> Alexandre now committed does not solve the problem for me, and my 
> autoconf foo is a bit rusty:
>
> cdrom.c: In function `CDROM_ScsiPassThroughDirect':
>   drom.c:1423: error: invalid application of `sizeof' to an incomplete type
>   cdrom.c: In function `CDROM_ScsiPassThrough':
>   cdrom.c:1543: error: invalid application of `sizeof' to an incomplete type

I don't see how it worked before then, you need to investigate why '#ifdef SENSEBUFLEN'
case doesn't work for you.
> Alexandre, please find below another patch which does not fix this either, 
> but gets rid of some unused variable warnings.
>
> Gerald
>
> ChangeLog:
> Avoid unused variable warnings in CDROM_ScsiPassThroughDir() and
> CDROM_ScsiPassThrough().

This means that HAVE_SCSIREQ_T_CMD is not defined for you, i.e. scsireq_t
has not been detected. Very likely this is caused by a wrong/missing header
includes in the configure.ac test, please fix it, it's easy enough.

[Gerald Pfeifer]

On Thu, 30 Jun 2005, Dmitry Timoshkov wrote:
> > Alexandre, please find below another patch which does not fix this either, 
> > but gets rid of some unused variable warnings.
> > ChangeLog:
> > Avoid unused variable warnings in CDROM_ScsiPassThroughDir() and
> > CDROM_ScsiPassThrough().
>
> This means that HAVE_SCSIREQ_T_CMD is not defined for you, i.e. scsireq_t
> has not been detected. Very likely this is caused by a wrong/missing header
> includes in the configure.ac test, please fix it, it's easy enough.

I found that FreeBSD does not have this type defined anywhere, so I hope 
Alexandre will take my patch.

Gerald

[Dmitry Timoshkov]

"Gerald Pfeifer"  wrote:
> > This means that HAVE_SCSIREQ_T_CMD is not defined for you, i.e. scsireq_t
> > has not been detected. Very likely this is caused by a wrong/missing header
> > includes in the configure.ac test, please fix it, it's easy enough.
>
> I found that FreeBSD does not have this type defined anywhere, so I hope 
> Alexandre will take my patch.

Any idea why it was in the '#elif defined(__FreeBSD__)' case then?

[Gerald Pfeifer]

On Thu, 30 Jun 2005, Dmitry Timoshkov wrote:
> > cdrom.c: In function `CDROM_ScsiPassThroughDirect':
> >   drom.c:1423: error: invalid application of `sizeof' to an incomplete type
> >   cdrom.c: In function `CDROM_ScsiPassThrough':
> >   cdrom.c:1543: error: invalid application of `sizeof' to an incomplete type
>
> I don't see how it worked before then, you need to investigate why 
> '#ifdef SENSEBUFLEN' case doesn't work for you.

I checked, and there is no occurrence of SENSEBUFLEN in /usr/include on
a SUSE 9.2 machine nor on FreeBSD nor in the entire Wine source tree.

How shall we proceed on this one?

Gerald

[Dmitry Timoshkov]

"Gerald Pfeifer"  wrote:
> > I don't see how it worked before then, you need to investigate why 
> > '#ifdef SENSEBUFLEN' case doesn't work for you.
>
> I checked, and there is no occurrence of SENSEBUFLEN in /usr/include on
> a SUSE 9.2 machine nor on FreeBSD nor in the entire Wine source tree.

Again, any idea why it was in the '#elif defined(__FreeBSD__)' case then?

[Gerald Pfeifer]

On Fri, 1 Jul 2005, Dmitry Timoshkov wrote:
> > > This means that HAVE_SCSIREQ_T_CMD is not defined for you, i.e. scsireq_t
> > > has not been detected. Very likely this is caused by a wrong/missing
> > > header
> >
> > I found that FreeBSD does not have this type defined anywhere
>
> Any idea why it was in the '#elif defined(__FreeBSD__)' case then?

No, sorry.

On Fri, 1 Jul 2005, Dmitry Timoshkov wrote:
> > I checked, and there is no occurrence of SENSEBUFLEN in /usr/include on
> > a SUSE 9.2 machine nor on FreeBSD nor in the entire Wine source tree.
>
> Again, any idea why it was in the '#elif defined(__FreeBSD__)' case then?

I don't really understand the point of your question: SENSEBUFLEN simply 
is not available on any system (Linux or FreeBSD) I checked, and the code
in question is not inside some #if defined(__FreeBSD__).

However, I think I may have a patch candidate for Alexandre.

Gerald