[Freeipa-devel] [PATCH] 0013 Fix SOA record update

[Adam Tkac]

On 03/22/2012 04:27 PM, Petr Spacek wrote:
> Hello,

Hello,

>
> this patch fixes wrong SOA updates during DNS zone dynamic update (via 
> nsupdate).
>
> It fixes LDAP plugin part of BZ:
> https://bugzilla.redhat.com/show_bug.cgi?id=805871

Good catch, the patch looks fine for me, please check my comments and 
then push it.

Regards, Adam

>
> Incorrect SOA serial number format after IPA installation has to be 
> solved in IPA install scripts.
>
> Petr^2 Spacek
>
> bind-dyndb-ldap-pspacek-0013-Fix-SOA-record-update.patch
>
>
>  From f40423b30a0d62dcef9ba4962e4b3d2ece1a0ded Mon Sep 17 00:00:00 2001
> From: Petr Spacek<pspacek at redhat.com>
> Date: Thu, 22 Mar 2012 16:15:56 +0100
> Subject: [PATCH] Fix SOA record update. Signed-off-by: Petr Spacek
>   <pspacek at redhat.com>
>
> ---
>   src/ldap_helper.c |   15 ++++++++++++---
>   1 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index 971d90f..bbdb388 100644
> --- a/src/ldap_helper.c
> +++ b/src/ldap_helper.c
> @@ -47,6 +47,7 @@
>   #include<isc/netaddr.h>
>   #include<isc/parseint.h>
>   #include<isc/timer.h>
> +#include<isc/string.h>
>
>   #include<alloca.h>
>   #define LDAP_DEPRECATED 1
> @@ -2167,6 +2168,7 @@ static isc_result_t
>   modify_soa_record(ldap_connection_t *ldap_conn, const char *zone_dn,
>   		  dns_rdata_t *rdata)
>   {
> +	isc_result_t result;
>   	isc_mem_t *mctx = ldap_conn->mctx;
>   	dns_rdata_soa_t soa;
>   	LDAPMod change[5];
> @@ -2175,13 +2177,16 @@ modify_soa_record(ldap_connection_t *ldap_conn, const char *zone_dn,
>   		NULL
>   	};
>
> +/* all values in SOA record are isc_uint32_t, i.e. max. 2^32-1 */
> +#define MAX_SOANUM_LENGTH 10
What about defining this to 11? After that you can omit "+ 1" additions 
below.
>   #define SET_LDAP_MOD(index, name) \
>   	change[index].mod_op = LDAP_MOD_REPLACE; \
>   	change[index].mod_type = "idnsSOA" #name; \
>   	change[index].mod_values = alloca(2 * sizeof(char *)); \
> -	change[index].mod_values[0] = alloca(sizeof(soa.name) + 1); \
> +	change[index].mod_values[0] = alloca((MAX_SOANUM_LENGTH + 1)*sizeof(char)); \
Although this is correct, I don't think that "*sizeof(char)" is needed, 
please remove it.
>   	change[index].mod_values[1] = NULL; \
> -	snprintf(change[index].mod_values[0], sizeof(soa.name) + 1, "%d", soa.name)
> +	CHECK(isc_string_printf(change[index].mod_values[0], \
> +		(MAX_SOANUM_LENGTH + 1)*sizeof(char), "%u", soa.name));
Ditto.
>
>   	dns_rdata_tostruct(rdata, (void *)&soa, mctx);
>
> @@ -2193,8 +2198,12 @@ modify_soa_record(ldap_connection_t *ldap_conn, const char *zone_dn,
>
>   	dns_rdata_freestruct((void *)&soa);
>
> -	return ldap_modify_do(ldap_conn, zone_dn, changep, ISC_FALSE);
> +	result = ldap_modify_do(ldap_conn, zone_dn, changep, ISC_FALSE);
> +
> +cleanup:
> +	return result;
>
> +#undef MAX_SOANUM_LENGTH
>   #undef SET_LDAP_MOD
>   }
>