On 14.5.2013 17:42, Tomas Babej wrote:
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

Here, have my ACK.

Pushed to master: 35ac3e422376cc28040d03c7550c2c68a967a0cf

Petr^2 Spacek

Freeipa-devel mailing list

Reply via email to