[PATCHES] lib/util - cleanups, build restructuring, charset compatibility

[Andreas Schneider]

On Wednesday 07 January 2015 14:17:23 Lukas Slebodnik wrote:
> On (07/01/15 14:56), Martin Schwenke wrote:
> >On Tue, 6 Jan 2015 17:21:20 -0800, Jeremy Allison <jra at samba.org> wrote:
> >> On Wed, Jan 07, 2015 at 06:26:47AM +1100, Martin Schwenke wrote:
> >> > Hi Lukas,
> >> > 
> >> > On Tue, 6 Jan 2015 15:24:16 +0100, Lukas Slebodnik
> >> > 
> >> > <lslebodn at redhat.com> wrote:
> >> > > Could you take a look into
> >> > > https://bugzilla.samba.org/show_bug.cgi?id=11033?
> >> > > 
> >> > > I might be wrong, but I suspect problem is caused by these patches.
> >> > 
> >> > Well, the problem is really caused by a dependency on broken public
> >> > headers that don't define what they use.  ;-)
> >> > 
> >> > samba_util.h is included and it uses DEBUG(), so DEBUG() really needs
> >> > to be defined.  Using Samba public headers that are well defined means
> >> 
> >> Does samba_util.h use DEBUG ? I don't see that in the file.
> >
> >Sorry, I was unclear and vague.  :-(
> >
> >samba_util.h used to define SMB_ASSERT() directly and SMB_ASSERT()
> >uses DEBUG().  I moved SMB_ASSERT() to fault.h when I created that file.
> >So I also added an include of debug.h in fault.h, and samba_util.h gets
> >it indirectly via fault.h.
> >
> >Having thought about it more, the simplest fix/workaround is to actually
> >make the include of debug.h in fault.h conditional.  If the including
> >code has already defined DEBUG() then we don't bother.
> >
> >Patch attached.  I've only compile-tested it but it seems
> >obvious...  :-)
> >
> >peace & happiness,
> >martin
> >
> >From e8bd0a8793f135a8d2b130932d68e6bad1fa7f7d Mon Sep 17 00:00:00 2001
> >From: Martin Schwenke <martin at meltin.net>
> >Date: Wed, 7 Jan 2015 14:47:29 +1100
> >Subject: [PATCH] lib/util: Conditionally include debug.h in fault.h
> >
> >This allows 3rd party users of the public library to define their own
> >DEBUG() macro.
> 
> With this patch, we will be able to workaround problem. Our macro will be
> defined before including header file <ndr.h>.
> The definition of our DEBUG macro is different, but that isn't problem for
> us because we do not use macro SMB_ASSERT.
> 
> The question is whether DEBUG macro need to be used in public macro
> SMB_ASSERT (fault.h). If yes it would be better to have prefixed version
> SMB_DEBUG in public header file and old style DEBUG macro just in samba
> private header file.

We should fix this probably as this will go into Samba 4.2. I've made the bug 
a blocker for 4.2.

Remember that OpenChange uses these headers too.


I think SMB_ASSERT should only be defined if the samba macro is available. 
This means debug.h should have #define SAMBA_DEBUG 1

#ifdef SAMBA_DEBUG
#define SMB_ASSERT() ...
#endif

Just my 2 cents ...

	-- andreas

-- 
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             asn at samba.org
www.samba.org