[libdvdcss-devel] Cygwin patches

[Diego Biurrun]

These patches have been sent before, but the discussion was sort
of inconclusive and now there is a fresh git repo. So here are
the last remaining patches from my set again, freshly rebased on
top of master.

Diego

[Diego Biurrun]

Also drop the now unnecessary direct.h configure.ac check. That header
file is no longer used and always available on Windows.

Signed-off-by: Diego Biurrun <***@biurrun.de>
---
configure.ac | 3 ---
msvc/config.h | 1 -
src/libdvdcss.c | 6 +-----
3 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/configure.ac b/configure.ac
index 7c92ce6..d938d64 100644
--- a/configure.ac
+++ b/configure.ac
@@ -48,9 +48,6 @@ CAN_BUILD_LIBDVDCSS=0

dnl for windoze
AC_CHECK_HEADERS(windows.h,[
- AC_CHECK_HEADERS(direct.h,,,[
- #include <windows.h>
- ])
AC_CHECK_HEADERS(winioctl.h,[
CAN_BUILD_LIBDVDCSS=1
],,[
diff --git a/msvc/config.h b/msvc/config.h
index ac88ea2..5a313ee 100644
--- a/msvc/config.h
+++ b/msvc/config.h
@@ -7,7 +7,6 @@
/* #undef DVD_STRUCT_IN_SYS_CDIO_H */
/* #undef DVD_STRUCT_IN_SYS_DVDIO_H */
/* #undef HAVE_BSD_DVD_STRUCT */
-#define HAVE_DIRECT_H 1
#define HAVE_DLFCN_H 1
/* #undef HAVE_DVD_H */
/* #undef HAVE_INTTYPES_H */
diff --git a/src/libdvdcss.c b/src/libdvdcss.c
index b10f598..a5b5bbb 100644
--- a/src/libdvdcss.c
+++ b/src/libdvdcss.c
@@ -120,10 +120,6 @@
# include <limits.h>
#endif

-#ifdef HAVE_DIRECT_H
-# include <direct.h>
-#endif
-
#include "dvdcss/dvdcss.h"

#include "common.h"
@@ -242,7 +238,7 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
*/
if( psz_cache == NULL || psz_cache[0] == '\0' )
{
-#ifdef HAVE_DIRECT_H
+#if defined(WIN32)
typedef HRESULT( WINAPI *SHGETFOLDERPATH )
( HWND, int, HANDLE, DWORD, LPTSTR );

[Jean-Baptiste Kempf]

Post by Diego Biurrun
Also drop the now unnecessary direct.h configure.ac check. That header
file is no longer used and always available on Windows.

But we use _mkdir, no?
So I think we need to include direct.h somewhere else in this file.

But I agree for the rest of the patch.

[Diego Biurrun]

Post by Jean-Baptiste Kempf

Post by Diego Biurrun
Also drop the now unnecessary direct.h configure.ac check. That header
file is no longer used and always available on Windows.

But we use _mkdir, no?
So I think we need to include direct.h somewhere else in this file.
But I agree for the rest of the patch.

I think that depends on MSVC support, doesn't it?

Does libdvdcss currently compile on MSVC and/or is MSVC support a goal?

Diego

[Jean-Baptiste Kempf]

Post by Diego Biurrun
I think that depends on MSVC support, doesn't it?

It does.

Post by Diego Biurrun
Does libdvdcss currently compile on MSVC and/or is MSVC support a goal?

It should compile, seeing the msvc/ folder. At least, there are no
strong reasons against it... And msvc/config.h should define
HAVE_DIRECT_H.

Best regards,

[Diego Biurrun]

Post by Jean-Baptiste Kempf

Post by Diego Biurrun
I think that depends on MSVC support, doesn't it?

It does.

Post by Diego Biurrun
Does libdvdcss currently compile on MSVC and/or is MSVC support a goal?

It should compile, seeing the msvc/ folder. At least, there are no
strong reasons against it... And msvc/config.h should define
HAVE_DIRECT_H.

It does not. So it "should" compile, but has anybody actually tested it?

Diego

[Diego Biurrun]

Post by Diego Biurrun

And msvc/config.h should define HAVE_DIRECT_H.

It does not. So it "should" compile, but has anybody actually tested it?

I got confused here. It does not define HAVE_DIRECT_H any longer because
I removed that define in the patch.

Do we need the check in configure or can we rely on direct.h being always
available on WIN32?

Diego

[Jean-Baptiste Kempf]

Post by Diego Biurrun
Do we need the check in configure or can we rely on direct.h being always
available on WIN32?

It will always be present, as far as we are concerned.
See http://msdn.microsoft.com/en-US/library/2fkk4dzw%28v=vs.80%29.aspx

Best regards,

[Diego Biurrun]

Signed-off-by: Diego Biurrun <***@biurrun.de>
---
configure.ac | 3 ---
src/ioctl.c | 41 +++++++++++++++++++----------------------
src/ioctl.h | 8 ++++++--
3 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/configure.ac b/configure.ac
index d938d64..ed03ae1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -26,9 +26,6 @@ case x"${target_os}" in
xdarwin*)
CFLAGS="${CFLAGS} -no-cpp-precomp"
;;
- x*cygwin*)
- AC_DEFINE(WIN32, 1, Using Win32.)
- ;;
xos2*)
LDFLAGS="-Zbin-files"
;;
diff --git a/src/ioctl.c b/src/ioctl.c
index b10c274..23296e5 100644
--- a/src/ioctl.c
+++ b/src/ioctl.c
@@ -31,14 +31,15 @@
/*****************************************************************************
* Preamble
*****************************************************************************/
-#include "config.h"
-
#include <stdio.h>
-
#include <string.h> /* memcpy(), memset() */
#include <sys/types.h>

-#if defined( WIN32 )
+#include "config.h"
+#include "common.h"
+#include "ioctl.h"
+
+#if defined( WIN32_DVD_API )
# include <windows.h>
# include <winioctl.h>
#elif defined ( __OS2__ )
@@ -90,10 +91,6 @@
# include <sys/dcmd_cam.h>
#endif

-#include "common.h"
-
-#include "ioctl.h"
-
/*****************************************************************************
* Local prototypes, BeOS specific
*****************************************************************************/
@@ -119,7 +116,7 @@ static int SolarisSendUSCSI( int fd, struct uscsi_cmd *p_sc );
/*****************************************************************************
* Local prototypes, win32 (aspi) specific
*****************************************************************************/
-#if defined( WIN32 )
+#if defined( WIN32_DVD_API )
static void WinInitSPTD ( SCSI_PASS_THROUGH_DIRECT *, int );
static void WinInitSSC ( struct SRB_ExecSCSICmd *, int );
static int WinSendSSC ( int, struct SRB_ExecSCSICmd * );
@@ -213,7 +210,7 @@ int ioctl_ReadCopyright( int i_fd, int i_layer, int *pi_copyright )

*pi_copyright = dvdbs.copyrightProtectionSystemType;

-#elif defined( WIN32 )
+#elif defined( WIN32_DVD_API )
if( WIN2K ) /* NT/2k/XP */
{
INIT_SPTD( GPCMD_READ_DVD_STRUCTURE, 8 );
@@ -373,7 +370,7 @@ int ioctl_ReadDiscKey( int i_fd, int *pi_agid, uint8_t *p_key )

memcpy( p_key, dvdbs.discKeyStructures, DVD_DISCKEY_SIZE );

-#elif defined( WIN32 )
+#elif defined( WIN32_DVD_API )
if( WIN2K ) /* NT/2k/XP */
{
DWORD tmp;
@@ -541,7 +538,7 @@ int ioctl_ReadTitleKey( int i_fd, int *pi_agid, int i_pos, uint8_t *p_key )

memcpy( p_key, dvdbs.titleKeyValue, DVD_KEY_SIZE );

-#elif defined( WIN32 )
+#elif defined( WIN32_DVD_API )
if( WIN2K ) /* NT/2k/XP */
{
DWORD tmp;
@@ -687,7 +684,7 @@ int ioctl_ReportAgid( int i_fd, int *pi_agid )

*pi_agid = dvdbs.grantID;

-#elif defined( WIN32 )
+#elif defined( WIN32_DVD_API )
if( WIN2K ) /* NT/2k/XP */
{
ULONG id;
@@ -808,7 +805,7 @@ int ioctl_ReportChallenge( int i_fd, int *pi_agid, uint8_t *p_challenge )

memcpy( p_challenge, dvdbs.challengeKeyValue, DVD_CHALLENGE_SIZE );

-#elif defined( WIN32 )
+#elif defined( WIN32_DVD_API )
if( WIN2K ) /* NT/2k/XP */
{
DWORD tmp;
@@ -940,7 +937,7 @@ int ioctl_ReportASF( int i_fd, int *pi_remove_me, int *pi_asf )

*pi_asf = dvdbs.successFlag;

-#elif defined( WIN32 )
+#elif defined( WIN32_DVD_API )
if( WIN2K ) /* NT/2k/XP */
{
DWORD tmp;
@@ -1075,7 +1072,7 @@ int ioctl_ReportKey1( int i_fd, int *pi_agid, uint8_t *p_key )

memcpy( p_key, dvdbs.key1Value, DVD_KEY_SIZE );

-#elif defined( WIN32 )
+#elif defined( WIN32_DVD_API )
if( WIN2K ) /* NT/2k/XP */
{
DWORD tmp;
@@ -1192,7 +1189,7 @@ int ioctl_InvalidateAgid( int i_fd, int *pi_agid )

i_ret = ioctl( i_fd, DKIOCDVDSENDKEY, &dvd );

-#elif defined( WIN32 )
+#elif defined( WIN32_DVD_API )
if( WIN2K ) /* NT/2k/XP */
{
DWORD tmp;
@@ -1320,7 +1317,7 @@ int ioctl_SendChallenge( int i_fd, int *pi_agid, uint8_t *p_challenge )

i_ret = ioctl( i_fd, DKIOCDVDSENDKEY, &dvd );

-#elif defined( WIN32 )
+#elif defined( WIN32_DVD_API )
if( WIN2K ) /* NT/2k/XP */
{
DWORD tmp;
@@ -1457,7 +1454,7 @@ int ioctl_SendKey2( int i_fd, int *pi_agid, uint8_t *p_key )

i_ret = ioctl( i_fd, DKIOCDVDSENDKEY, &dvd );

-#elif defined( WIN32 )
+#elif defined( WIN32_DVD_API )
if( WIN2K ) /* NT/2k/XP */
{
DWORD tmp;
@@ -1603,7 +1600,7 @@ int ioctl_ReportRPC( int i_fd, int *p_type, int *p_mask, int *p_scheme )
*p_mask = dvdbs.driveRegion;
*p_scheme = dvdbs.rpcScheme;

-#elif defined( WIN32 )
+#elif defined( WIN32_DVD_API )
if( WIN2K ) /* NT/2k/XP */
{
DWORD tmp;
@@ -1746,7 +1743,7 @@ int ioctl_SendRPC( int i_fd, int i_pdrc )

i_ret = ioctl( i_fd, DKIOCDVDSENDKEY, &dvd );

-#elif defined( WIN32 )
+#elif defined( WIN32_DVD_API )
if( WIN2K ) /* NT/2k/XP */
{
INIT_SPTD( GPCMD_SEND_KEY, 8 );
@@ -1969,7 +1966,7 @@ static int SolarisSendUSCSI( int i_fd, struct uscsi_cmd *p_sc )
}
#endif

-#if defined( WIN32 )
+#if defined( WIN32_DVD_API )
/*****************************************************************************
* WinInitSPTD: initialize a sptd structure
*****************************************************************************
diff --git a/src/ioctl.h b/src/ioctl.h
index 6f33d5b..683b885 100644
--- a/src/ioctl.h
+++ b/src/ioctl.h
@@ -24,6 +24,10 @@
#ifndef DVDCSS_IOCTL_H
#define DVDCSS_IOCTL_H

+#if defined( WIN32 ) || defined( __CYGWIN__ )
+# define WIN32_DVD_API
+#endif
+
int ioctl_ReadCopyright ( int, int, int * );
int ioctl_ReadDiscKey ( int, int *, uint8_t * );
int ioctl_ReadTitleKey ( int, int *, int, uint8_t * );
@@ -101,7 +105,7 @@ int ioctl_SendRPC ( int, int );
/*****************************************************************************
* Common macro, win32 specific
*****************************************************************************/
-#if defined( WIN32 )
+#if defined( WIN32_DVD_API )
#define INIT_SPTD( TYPE, SIZE ) \
DWORD tmp; \
SCSI_PASS_THROUGH_DIRECT sptd; \
@@ -194,7 +198,7 @@ typedef union dvd_authinfo dvd_authinfo;
/*****************************************************************************
* win32 ioctl specific
*****************************************************************************/
-#if defined( WIN32 )
+#if defined( WIN32_DVD_API )

#define WIN32_LEAN_AND_MEAN
#include <windows.h>

[Jean-Baptiste Kempf]

Post by Diego Biurrun
---
configure.ac | 3 ---
src/ioctl.c | 41 +++++++++++++++++++----------------------
src/ioctl.h | 8 ++++++--
3 files changed, 25 insertions(+), 27 deletions(-)

To be honest, I don't understand the reasoning and the necessity of this
patch.

[Diego Biurrun]

Post by Jean-Baptiste Kempf

Post by Diego Biurrun
---
configure.ac | 3 ---
src/ioctl.c | 41 +++++++++++++++++++----------------------
src/ioctl.h | 8 ++++++--
3 files changed, 25 insertions(+), 27 deletions(-)

To be honest, I don't understand the reasoning and the necessity of this
patch.

The log message needs to be updated to make this all clearer.

In any case, Cygwin does not naturally define WIN32 and should not be
grouped with MinGW and MSVC as it attempts to be a POSIX system.
Accessing hardware under Windows is a different thing though.
It seems cleaner to group Cygwin together with the WIN32 systems
for that case instead of globally.

Diego

[Jean-Baptiste Kempf]

Post by Diego Biurrun
It seems cleaner to group Cygwin together with the WIN32 systems
for that case instead of globally.

Why not define WIN32 then, in this CYGWIN case?

I mean there is just a difference of include in CYGWIN case, the rest
uses the same path everywhere.

Best regards,

[Diego Biurrun]

Post by Jean-Baptiste Kempf

Post by Diego Biurrun
It seems cleaner to group Cygwin together with the WIN32 systems
for that case instead of globally.

Why not define WIN32 then, in this CYGWIN case?
I mean there is just a difference of include in CYGWIN case, the rest
uses the same path everywhere.

No, there is a much larger difference, look at all the WIN32 ifdefs
around the code. I believe it's much safer to enable this specific
case for Cygwin instead of grouping Cygwin together with WIN32, which
is not correct and thus brittle. We might stumble over other issues
in the future.

Diego

[Jean-Baptiste Kempf]

Post by Diego Biurrun

Post by Jean-Baptiste Kempf

Post by Diego Biurrun
It seems cleaner to group Cygwin together with the WIN32 systems
for that case instead of globally.

Why not define WIN32 then, in this CYGWIN case?
I mean there is just a difference of include in CYGWIN case, the rest
uses the same path everywhere.

No, there is a much larger difference, look at all the WIN32 ifdefs
around the code. I believe it's much safer to enable this specific

Yes, and Cygwin should use all of them, except the 2 io include and the
struct. See ifdef(WIN32) && !CYGWIN.

Post by Diego Biurrun
case for Cygwin instead of grouping Cygwin together with WIN32, which
is not correct and thus brittle. We might stumble over other issues
in the future.

Sorry, but using Cygwin without Win32 headers to access Win32 devices
is the most stupid thing I have ever heard, since a loong time...

Best regards,

[Diego Biurrun]

Also drop the now unnecessary direct.h configure.ac check.
That header file is always available on Windows.

Signed-off-by: Diego Biurrun <***@biurrun.de>
---

This should be the non-controversial part of my patchset that
can be applied right away, rebased on master so as not to
depend on the other patches.

configure.ac | 3 ---
msvc/config.h | 1 -
src/libdvdcss.c | 9 +++------
3 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/configure.ac b/configure.ac
index 7c92ce6..d938d64 100644
--- a/configure.ac
+++ b/configure.ac
@@ -48,9 +48,6 @@ CAN_BUILD_LIBDVDCSS=0

dnl for windoze
AC_CHECK_HEADERS(windows.h,[
- AC_CHECK_HEADERS(direct.h,,,[
- #include <windows.h>
- ])
AC_CHECK_HEADERS(winioctl.h,[
CAN_BUILD_LIBDVDCSS=1
],,[
diff --git a/msvc/config.h b/msvc/config.h
index ac88ea2..5a313ee 100644
--- a/msvc/config.h
+++ b/msvc/config.h
@@ -7,7 +7,6 @@
/* #undef DVD_STRUCT_IN_SYS_CDIO_H */
/* #undef DVD_STRUCT_IN_SYS_DVDIO_H */
/* #undef HAVE_BSD_DVD_STRUCT */
-#define HAVE_DIRECT_H 1
#define HAVE_DLFCN_H 1
/* #undef HAVE_DVD_H */
/* #undef HAVE_INTTYPES_H */
diff --git a/src/libdvdcss.c b/src/libdvdcss.c
index b10f598..2bbb2dc 100644
--- a/src/libdvdcss.c
+++ b/src/libdvdcss.c
@@ -120,10 +120,6 @@
# include <limits.h>
#endif

-#ifdef HAVE_DIRECT_H
-# include <direct.h>
-#endif
-
#include "dvdcss/dvdcss.h"

#include "common.h"
@@ -132,7 +128,8 @@
#include "ioctl.h"
#include "device.h"

-#if defined(WIN32)
+#if defined(WIN32) && !defined(__CYGWIN__)
+#include <direct.h>
#define mkdir(a, b) _mkdir(a)
#endif

@@ -242,7 +239,7 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
*/
if( psz_cache == NULL || psz_cache[0] == '\0' )
{
-#ifdef HAVE_DIRECT_H
+#if defined(WIN32) && !defined(__CYGWIN__)
typedef HRESULT( WINAPI *SHGETFOLDERPATH )
( HWND, int, HANDLE, DWORD, LPTSTR );