On Mon, Sep 24, 2012 at 03:27:13PM +0200, Petr Spacek wrote:
> On 09/24/2012 03:15 PM, Adam Tkac wrote:
> >On Mon, Sep 17, 2012 at 05:07:44PM +0200, Petr Spacek wrote:
> >>Hello,
> >>
> >>this patch adds DN to log message about improperly formated Resource 
> >>Records.
> >
> >Hi,
> >
> >please check my comment below, otherwise ack.
> >
> >Regards, Adam
> >
> >> From d36ae54c593c33a45ef9936720357ff7de30c8b5 Mon Sep 17 00:00:00 2001
> >>From: Petr Spacek <pspa...@redhat.com>
> >>Date: Mon, 17 Sep 2012 17:01:41 +0200
> >>Subject: [PATCH] Improve log message about improperly formated Resource
> >>  Records.
> >>
> >>Signed-off-by: Petr Spacek <pspa...@redhat.com>
> >>---
> >>  src/ldap_helper.c | 17 ++++++++++-------
> >>  1 file changed, 10 insertions(+), 7 deletions(-)
> >>
> >>diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> >>index 
> >>d22e8714df57edaad4cf45e6cc26ec0dbbd59108..2245cb982f26eab165a327b4ad72046f9eb4024e
> >> 100644
> >>--- a/src/ldap_helper.c
> >>+++ b/src/ldap_helper.c
> >>@@ -1473,6 +1473,8 @@ ldap_parse_rrentry(isc_mem_t *mctx, ldap_entry_t 
> >>*entry,
> >>    dns_rdata_t *rdata = NULL;
> >>    dns_rdatalist_t *rdlist = NULL;
> >>    ldap_attribute_t *attr;
> >>+   const char *dn = "<NULL entry>";
> >>+   const char *data = "<NULL data>";
> >>
> >>    result = add_soa_record(mctx, qresult, origin, entry,
> >>                            rdatalist, fake_mname);
> >>@@ -1501,6 +1503,11 @@ ldap_parse_rrentry(isc_mem_t *mctx, ldap_entry_t 
> >>*entry,
> >>    return ISC_R_SUCCESS;
> >>
> >>  cleanup:
> >>+   if (entry != NULL)
> >>+           dn = entry->dn;
> >>+   if (buf != NULL && str_buf(buf) != NULL)
> >>+           data = str_buf(buf);
> >>+   log_error_r("failed to parse RR entry: dn '%s': data '%s'", dn, data);
> >>    return result;
> >>  }
> >>
> >>@@ -1539,7 +1546,6 @@ ldapdb_nodelist_get(isc_mem_t *mctx, ldap_instance_t 
> >>*ldap_inst, dns_name_t *nam
> >>            dns_name_init(&node_name, NULL);
> >>            if (dn_to_dnsname(mctx, entry->dn,  &node_name, NULL)
> >>                != ISC_R_SUCCESS) {
> >>-                   log_error("Failed to parse dn %s", entry->dn);
> >
> >I prefer to keep this error in place. Otherwise "bad" DNs will be silently
> >skipped.
> 
> There is another log_error_r() call in dn_to_dnsname(). Actually old
> code logs error two times and the message from dn_to_dnsname() is
> more verbose.

Thanks for clarification, ack for the original version.

> 
> >
> >>                    continue;
> >>            }
> >>
> >>@@ -1551,7 +1557,6 @@ ldapdb_nodelist_get(isc_mem_t *mctx, ldap_instance_t 
> >>*ldap_inst, dns_name_t *nam
> >>                                   string, &node->rdatalist);
> >>            }
> >>            if (result != ISC_R_SUCCESS) {
> >>-                   log_error("Failed to parse RR entry (%s)", 
> >>str_buf(string));
> >>                    /* node cleaning */     
> >>                    dns_name_reset(&node->owner);
> >>                    ldapdb_rdatalist_destroy(mctx, &node->rdatalist);
> >>@@ -1609,11 +1614,9 @@ ldapdb_rdatalist_get(isc_mem_t *mctx, 
> >>ldap_instance_t *ldap_inst, dns_name_t *na
> >>    for (entry = HEAD(ldap_qresult->ldap_entries);
> >>            entry != NULL;
> >>            entry = NEXT(entry, link)) {
> >>-           if (ldap_parse_rrentry(mctx, entry, ldap_qresult,
> >>-                                  origin, ldap_inst->fake_mname,
> >>-                                  string, rdatalist) != ISC_R_SUCCESS ) {
> >>-                   log_error("Failed to parse RR entry (%s)", 
> >>str_buf(string));
> >>-           }
> >>+           CHECK(ldap_parse_rrentry(mctx, entry, ldap_qresult,
> >>+                              origin, ldap_inst->fake_mname,
> >>+                              string, rdatalist));
> >>    }
> >>
> >>    if (!EMPTY(*rdatalist)) {
> >>--
> >>1.7.11.4
> >>
> >
> >
> 

-- 
Adam Tkac, Red Hat, Inc.

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to