[SCM] Samba Shared Repository - branch master updated

Matthias Dieter Wallnöfer mdw at samba.org
Sun Dec 12 14:50:45 MST 2010


Hi Jelmer,

Jelmer Vernooij wrote:
> The same goes for that macro - if it doesn't deal with proper free'ing,
> then why not avoid it rather than rewrite the rest of the function that
> uses it?
>
> discard_const_p is bad, and we should avoid it unless we really can.
>    
well, I agree that "discard_const_p" isn't so nice. But I think the 
right solution (if we make use of memory contexts) would be to never 
derive them from NULL, but from a "self" instance or kind of this or if 
not possible (static) the LDB context.
>    
>>>>>> diff --git a/source4/lib/ldb/pyldb_util.c b/source4/lib/ldb/pyldb_util.c
>>>>>> index 3e015d0..35071f3 100644
>>>>>> --- a/source4/lib/ldb/pyldb_util.c
>>>>>> +++ b/source4/lib/ldb/pyldb_util.c
>>>>>> @@ -23,10 +23,7 @@
>>>>>>        License along with this library; if not, see<http://www.gnu.org/licenses/>.
>>>>>>     */
>>>>>>
>>>>>> -#include<Python.h>
>>>>>> -#include "replace.h"
>>>>>>     #include "pyldb.h"
>>>>>> -#include<ldb.h>
>>>>>>
>>>>>>     static PyObject *ldb_module = NULL;
>>>>>>
>>>>>>
>>>>>>              
>>>>> See above. Python.h is included for a reason. Also, replace.h might not
>>>>> be necessary on your system but necessary on others (as some
>>>>> functionality is not provided by the OS).
>>>>>
>>>>>            
>>>> But let me think - we include system headers only by libreplace. So at
>>>> the end it doesn't matter if replace or a system header defines it.
>>>> Obviously there we have no need for a system call - otherwise the build
>>>> would have broken.
>>>>
>>>>          
>>> replace.h isn't the only way in which we get system headers, e.g.
>>> Python.h also includes a bunch - at least stdlib.h, unistd.h, stddef.h,
>>> string.h, stdio.h, limits.h and assert.h.
>>>
>>>        
>> In this special case we don't need the "replace.h" anymore since I do
>> now include "ldb_private.h" (which itself includes it).
>>      
> Is there any particular reason why pyldb_util.c requires ldb_private.h ?
> We should avoid including it if we can (and thus avoid tying pyldb_util
> to a specific version of ldb).
>    
Okay.
>> PS: Could you please start the implementation of the other message
>> attribute-mapping function in pyldb?
>>      
> Sorry, that's on my todo list as are several other things. I'd be happy
> to review a patch that adds it though.
>    
Okay.

Cheers,
Matthias


More information about the samba-technical mailing list