[Freeipa-devel] [PATCH 0149] Clean up PTR record synchronization code and make it more robust
Petr Spacek
pspacek at redhat.com
Mon May 13 14:32:32 UTC 2013
On 13.5.2013 15:23, Lukas Slebodnik wrote:
> 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);
>> + }
Thank you for catching this! Nobody noticed it for one and half year :-)
In any case, this code can't handle IPv6 addresses. We will triage it
tomorrow: https://fedorahosted.org/bind-dyndb-ldap/ticket/118
Fixed patch is attached. The new version includes also typo fix: " could" =>
"could".
--
Petr Spacek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bind-dyndb-ldap-pspacek-0149-3-Clean-up-PTR-record-synchronization-code-and-make-it.patch
Type: text/x-patch
Size: 19022 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130513/bb066dbe/attachment.bin>
More information about the Freeipa-devel
mailing list