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

On Tue, Nov 18, 2014 at 08:03:42PM +0300, Evgeny Grin wrote:
> 18.11.2014, 19:51, "Diego Biurrun" <[email protected]>:
> > On Tue, Nov 18, 2014 at 06:46:43PM +0300, Evgeny Grin (Karlson2k) wrote:
> >>  From: Evgeny Grin <[email protected]>
> >>
> >>  ---
> >>   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.

> >>  --- 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.
> 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
_______________________________________________
libdvdcss-devel mailing list
[email protected]
https://mailman.videolan.org/listinfo/libdvdcss-devel