[Freeipa-devel] [PATCH 0149] Clean up PTR record synchronization code and make it more robust

[Lukas Slebodnik]

On (06/05/13 14:03), Petr Spacek wrote:
>On 18.4.2013 11:04, Petr Spacek wrote:
>>Hello,
>>
>>Clean up PTR record synchronization code and make it more robust.
>>
>>PTR record synchronization was split to smaller functions.
>>Input validation, error handling and logging was improved
>>significantly.
>
>Tbabej's GCC cries about uninitialized variable 'ptr_a_equal', but we
>weren't able to find any real error.
>
>This version of the patch contains a workaround for the GCC oddities.
>
>-- 
>Petr^2 Spacek

>From 5e6abb29df58ce00ecf7045254dfc7fb09fc4650 Mon Sep 17 00:00:00 2001
>From: Petr Spacek <pspacek at redhat.com>
>Date: Tue, 16 Apr 2013 16:10:09 +0200
>Subject: [PATCH] Clean up PTR record synchronization code and make it more
> robust.
>
>PTR record synchronization was split to smaller functions.
>Input validation, error handling and logging was improved
>significantly.
>
>Signed-off-by: Petr Spacek <pspacek at redhat.com>
>---
> src/ldap_helper.c | 507 ++++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 342 insertions(+), 165 deletions(-)
>
>diff --git a/src/ldap_helper.c b/src/ldap_helper.c
>index 8448412b7a1a9150bd24d9ca46575c0402be0c9f..6c5cf2e79d762251954e3bb099dbef98a0b2d805 100644
>--- a/src/ldap_helper.c
>+++ b/src/ldap_helper.c
>@@ -2830,35 +2830,360 @@ cleanup:
> #undef SET_LDAP_MOD
> }
> 
>+
>+#define SYNCPTR_PREF    "PTR record synchronization "
>+#define SYNCPTR_FMTPRE  SYNCPTR_PREF "(%s) for A/AAAA '%s' "
>+#define SYNCPTR_FMTPOST ldap_modop_str(mod_op), a_name_str
>+
>+static const char *
>+ldap_modop_str(unsigned int mod_op) {
>+	static const char add[] = "addition";
>+	static const char del[] = "deletion";
                       ^^^^^
Declaration(definition) of string should be consistent everywhere in the source code.
Please change "char _name[]" to "char * _name"
Sorry for nitpicking, I know that semantically is it the same,
but in the other places in source code, strings are
defined like "(const)? char *".
If you don't believe me, you can run next command :-)

    grep -Rn -E 'char.*=.*"[^"]*"' <path_to_bind-dyndb-ldap>

[snip]

>+static isc_result_t
>+ldap_find_ptr(ldap_instance_t *ldap_inst, const char *ip_str,
>+	      dns_name_t *ptr_name, ld_string_t *ptr_dn,
>+	      dns_name_t *zone_name) {
>+	isc_result_t result;
>+	isc_mem_t *mctx = ldap_inst->mctx;
>+
>+	in_addr_t ip;
>+
>+	/* Get string with IP address from change request
>+	 * and convert it to in_addr structure. */
>+	if ((ip = inet_addr(ip_str)) t) == 0) {
                                    ^^^^
If the input is invalid, INADDR_NONE (usually -1) is returned.
For details: man inet_addr

>+		log_bug(SYNCPTR_PREF " could not convert IP address "
>+			"from string '%s'", ip_str);
>+		CLEANUP_WITH(ISC_R_UNEXPECTED);
>+	}

LS