On 05/14/2013 02:01 PM, Petr Spacek wrote:
On 14.5.2013 11:45, Tomas Babej wrote:
On 05/10/2013 04:57 PM, Petr Spacek wrote:
On 6.5.2013 17:40, Tomas Hozza wrote:
On 04/08/2013 07:45 PM, Petr Spacek wrote:
Generalize attribute_name<->rdata_type conversions.

Attribute names are generated on-the-fly: String "Record" is appended
to textual representation of DNS RDATA type.

String "Record" is cut down from the attribute name during
attribute name to rdata type conversion.

 From now, the plugin doesn't add artificial limitation to supported
record types.


The patch looks good. (I didn't do functional test)

Cosmetic issue:
I think it would be good to dynamically allocate "mod_type" in LDAPMod
in every case and include the "mod_type" memory freeing in
free_ldapmod() function. Now one has to be be careful when it is
statically or dynamically allocated. Before it was static in every case.

It is good idea. This version of the patch contains ldap_mod_create()
function which allocates the whole structure including mod_type of fixed size. All writes to mod_type checks the array length, so it should not cause
any harm.

The function modify_soa_record() still uses statically allocated LDAPMod structure with statically allocated strings for mod_type, but the LDAPMod structure never leave this function. There are no calls to ldap_mod_create()
and ldap_mod_free(), so I think it is obvious.

Tbabej, please try to dynamically update some A records with sync_ptr
enabled. (And of course the support for some new type, like TLSA.)
For the existing record types, the patch works fine.

For any new types, a schema change is still required, since record types are
still hardcoded in LDAP schema:

  LDAP error: Object class violation: attribute "tlsarecord" not allowed

That is expected behaviour. The point is that schema change is much simpler than recompiling the bind-dyndb-ldap (and can be done at run-time).

BTW schema file contains instructions how to add support for any record type in a way compatible with rest of the world:

Was it ACK?

I was not nacking, just pointing out. Tested with changed schema, works as expected.

Here, have my ACK.


Freeipa-devel mailing list

Reply via email to