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
