[libdvdcss-devel] [PATCH] Do not include huge "windows.h" in main header, use with single typedef

[Evgeny Grin (Karlson2k)]

From: Evgeny Grin <***@kodi.tv>

---
src/libdvdcss.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/libdvdcss.h b/src/libdvdcss.h
index 5f6cb3e..335640e 100644
--- a/src/libdvdcss.h
+++ b/src/libdvdcss.h
@@ -27,8 +27,7 @@
#include <limits.h>

#ifdef WIN32
-# include "config.h"
-# include <windows.h>
+ typedef void* HANDLE; /* forward declaration */
#endif

#include "dvdcss/dvdcss.h"

[Diego Biurrun]

Post by Evgeny Grin (Karlson2k)
---
src/libdvdcss.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

Why? I don't see why we should duplicate any Windows API internals.

Post by Evgeny Grin (Karlson2k)
--- a/src/libdvdcss.h
+++ b/src/libdvdcss.h
@@ -27,8 +27,7 @@
#include <limits.h>
#ifdef WIN32
-# include "config.h"
-# include <windows.h>
+ typedef void* HANDLE; /* forward declaration */
#endif

The style is not OK; the file uses 4 spaces as indentation.

Diego

[Evgeny Grin]

 ---
  src/libdvdcss.h | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

Why?  I don't see why we should duplicate any Windows API internals.

Because other apps that include libdvdcss header usually do not require Win32 API.
I case of big projects this can significantly slowdown compilation.
Moreover, it can even break builds as win32 headers use a lot of defines like "#define GetUsername GetUsernameA". So if some app will include libdvdcss header in some file .cpp which use someClass::GetUsername(), it will produce problems.

 --- a/src/libdvdcss.h
 +++ b/src/libdvdcss.h
  #include <limits.h>
  #ifdef WIN32
 -#    include "config.h"
 -#    include <windows.h>
 +    typedef void* HANDLE; /* forward declaration */
  #endif

The style is not OK; the file uses 4 spaces as indentation.

It uses 4 spaces, but don't use "#" at start of the line.
But patch is wrong, "config.h" should be preserved.
Including "config.h" is another problem as it will include wrong "config.h" when libdvdcss is included in other project.
Typical solution is using "libdvdcss_config.h" with main header.

[Diego Biurrun]

Post by Evgeny Grin

 ---
  src/libdvdcss.h | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

Why?  I don't see why we should duplicate any Windows API internals.

Because other apps that include libdvdcss header usually do not require Win32 API.
I case of big projects this can significantly slowdown compilation.
Moreover, it can even break builds as win32 headers use a lot of defines like "#define GetUsername GetUsernameA". So if some app will include libdvdcss header in some file .cpp which use someClass::GetUsername(), it will produce problems.

libdvdcss.h is not a public header. If you #include it somewhere in your
application, then that is *wrong*. The only public header we have is
dvdcss/dvdcss.h.

Post by Evgeny Grin

 --- a/src/libdvdcss.h
 +++ b/src/libdvdcss.h
  #include <limits.h>
  #ifdef WIN32
 -#    include "config.h"
 -#    include <windows.h>
 +    typedef void* HANDLE; /* forward declaration */
  #endif

The style is not OK; the file uses 4 spaces as indentation.

It uses 4 spaces, but don't use "#" at start of the line.
But patch is wrong, "config.h" should be preserved.
Including "config.h" is another problem as it will include wrong "config.h" when libdvdcss is included in other project.
Typical solution is using "libdvdcss_config.h" with main header.

No, the solution is not to #include config.h in public headers, which we do.

Diego

[Diego Biurrun]

Post by Evgeny Grin

 ---
  src/libdvdcss.h | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

Why?  I don't see why we should duplicate any Windows API internals.

Because other apps that include libdvdcss header usually do not require Win32 API.
I case of big projects this can significantly slowdown compilation.

If you can suggest a smaller header, then I'm all ears. Duplicating the
typedef will not be accepted.

Diego

[Diego Biurrun]

Post by Diego Biurrun

Post by Evgeny Grin

 ---
  src/libdvdcss.h | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

Why?  I don't see why we should duplicate any Windows API internals.

Because other apps that include libdvdcss header usually do not require Win32 API.
I case of big projects this can significantly slowdown compilation.

If you can suggest a smaller header, then I'm all ears. Duplicating the
typedef will not be accepted.

Note that we already #include windows.h in other headers like ioctl.h,
but with '#define WIN32_LEAN_AND_MEAN' before it, maybe that will help.

Diego

[Jean-Baptiste Kempf]

Post by Diego Biurrun

Post by Diego Biurrun

Post by Evgeny Grin

 ---
  src/libdvdcss.h | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

Why?  I don't see why we should duplicate any Windows API internals.

Because other apps that include libdvdcss header usually do not require Win32 API.
I case of big projects this can significantly slowdown compilation.

If you can suggest a smaller header, then I'm all ears. Duplicating the
typedef will not be accepted.

Note that we already #include windows.h in other headers like ioctl.h,
but with '#define WIN32_LEAN_AND_MEAN' before it, maybe that will help.

This is why, it should be included only once, in common.h

With my kindest regards,