On Thu, Jul 19, 2012 at 01:59:01PM +0200, Petr Spacek wrote: > Hello, > > I have to explain my motivation behind INSIST a bit. Please see comments > below. > > On 07/19/2012 01:43 PM, Adam Tkac wrote: > >>On Wed, Jul 18, 2012 at 01:32:10PM +0200, Petr Spacek wrote: > >>+ CHECK(ldap_query(inst, conn, &ldap_qresult_zone, pevent->dn, > >>>+ LDAP_SCOPE_BASE, attrs_zone, 0, > >>> "(&(objectClass=idnsZone)(idnsZoneActive=TRUE))")); > This LDAP query has search scope LDAP_SCOPE_BASE. If I understood > LDAP correctly, it can return 0 or 1 result, but no more. (Two > objects can't have exactly same DN.) > > If specified LDAP query returned (or is believed to returned) more > than single result, then something went terribly wrong (memory > corruption/incorrect pointer arithmetic?). > > Theoretically this situation should never happen and I can remove > this check completely, if you want. > > >>> > >>>- for (entry = HEAD(ldap_qresult->ldap_entries); > >>>- entry != NULL; > >>>- entry = NEXT(entry, link)) { > >>>+ for (entry_zone = HEAD(ldap_qresult_zone->ldap_entries); > >>>+ entry_zone != NULL; > >>>+ entry_zone = NEXT(entry_zone, link)) { > >>> delete = ISC_FALSE; > >>> result = ldap_parse_zoneentry(entry, inst); > >>> if (result != ISC_R_SUCCESS) > >>> goto cleanup; > >>>+ > >>>+ if (PSEARCH_MODDN(pevent->chgtype)) { > >>>+ if (dn_to_dnsname(inst->mctx, pevent->prevdn, > >>>&prevname, NULL) > >>>+ == ISC_R_SUCCESS) { > >>>+ CHECK(ldap_delete_zone(inst, pevent->prevdn, > >>>ISC_TRUE)); > >>>+ } else { > >>>+ log_debug(5, "update_action: old zone wasn't > >>>managed " > >>>+ "by plugin, dn '%s'", > >>>pevent->prevdn); > >>>+ } > >>>+ > >>>+ /* fill the cache with records from renamed zone */ > >>>+ CHECK(ldap_query(inst, conn, &ldap_qresult_record, > >>>pevent->dn, > >>>+ LDAP_SCOPE_ONELEVEL, attrs_record, 0, > >>>+ "(objectClass=idnsRecord)")); > >>>+ > >>>+ /* LDAP schema requires SOA record (at least) */ > >>>+ INSIST(HEAD(ldap_qresult_record->ldap_entries) != NULL); > >>>+ for (entry_record = > >>>HEAD(ldap_qresult_record->ldap_entries); > >>>+ entry_record != NULL; > >>>+ entry_record = NEXT(entry_record, > >>>link)) { > >>>+ > >>>+ psearch_update(inst, entry_record, NULL); > >>>+ } > >>>+ } > >>>+ > >>>+ INSIST(NEXT(entry_zone, link) == NULL); /* no multiple zones > >>>with same DN */ > >This INSIST seems like too strong for me. Imagine situation when > >administrator > >wrongly modifies LDAP database and creates duplicated zone. In this case we > >will > AFAIK it is not possible, because idnsName attribute is part of DN > and duplicate DN is not allowed.
Ok, this makes sence. So push the patchset as is, thank you. Regards, Adam > >crash. I would prefer to log error here, that multiple zones exist and only > >the > >first occurence is used. > > > >>> } > > Petr^2 Spacek > -- Adam Tkac, Red Hat, Inc. _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel