Re: [389-devel] Please review: Plugin Default Config Entry

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Noriko Hosoi wrote:
Plugin Default Config Entry

Design doc:
http://directory.fedoraproject.org/wiki/Entry_USN#Plugin_Default_Config_Entry

New slapi APIs in libslapd:
  int slapi_set_plugin_default_config(char *type, char *attr);
  Description: Set given "type: attr" to the plugin default config entry
               (cn=plugin default config,cn=config).
  Parameters: type - Attribute type to add to the default config entry
              attr - Attribute value to add to the default config entry
  Return Value: 0 if the operation was successful
                non-0 if the operation was not successful

  int slapi_get_plugin_default_config(char *type, char ***attrs);
  Description: Get attribute values of given type from the plugin default
               config entry (cn=plugin default config,cn=config).
  Parameters: type - Attribute type to get from the default config entry
              attrs - Pointer to the string array to store the values of
                      the attribute
  Return Value: 0 if the operation was successful
              non-0 if the operation was not successful
  warning: Caller is responsible to free attrs by slapi_ch_array_free

Changes in the Replication plugin:
1) Functions to set replicated attributes
       agmt_set_replicated_attributes_from_attr and
       agmt_set_replicated_attributes_from_entry
   call _agmt_set_default_fractional_attrs to sets the default excluded
   attribute list from the plugin default config entry before setting
   them from each replication agreement.
   To support it, agmt_parse_excluded_attrs_config_attr is changed to be
   re-entrant.
2) Fixed a minor memory leak in the fractional attributes (ra->frac_attrs).
3) Added a check for the duplicated fractional attributes.

Changes in the USN plugin:
usn_start calls slapi_get_plugin_default_config to check if "entryusn" is
in the EXCLUDE list of the value of nsds5ReplicatedAttributeList in the
plugin default config entry or not.  If it does not exist, the function
adds it.  Note: If the nsds5ReplicatedAttributeList value exists and
"entryusn" is not in the EXCLUDE list, we have to append it to the list
instead of replacing it.

Thanks,
--noriko
------------------------------------------------------------------------

--
389-devel mailing list
389-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/fedora-directory-devel
About this code:
    if (new_attrs)
    {
-        *attrs = new_attrs;
+        charray_merge_nodup(attrs, new_attrs, 0);
+        slapi_ch_free((void **)&new_attrs);
    }

the 0 flag to charray_merge_nodup() means "do not copy" - so the strings in new_attrs will be moved to attrs, if they are not duplicates. If they are duplicates, they will not be moved - how will they be freed? There's no way for the caller of charray_merge_nodup to know which strings were moved and which were not (except by comparing pointers in attrs with new_attrs).

+            int i;
+            const char *val = slapi_value_get_string(sval);
+            for (i = slapi_attr_first_value(attr, &sval);

I don't think you should be calling slapi_value_get_string() here since sval is not initialized yet.

I think slapi_set_plugin_default_config() should always to a modify/add, and just ignore "type or value exists" errors. Or support single valued attributes only. Or allow the user to specify to add or replace in the case of a multi-valued attribute. What if the user modifies the list directly over LDAP or by editing dse.ldif? An internal call to set the value will wipe out their settings. Also, there is a reference to frational attr in the slapi_set_plugin_default_config() code.

I think slapi_set_plugin_default_config() will leak the pblock used for the initial search if rc != LDAP_SUCCESS or entries is NULL or empty.

slapi_get_plugin_default_config() - since you are assuming that the values of the requested attribute are all null terminated strings (since you are using slapi_value_get_string()), you could just use slapi_entry_attr_get_charray() to get all of the values as a char **.

Why does slapi_set_plugin_default_config() take a char * argument i.e. setting an attribute with a single string value, but slapi_get_plugin_default_config() returns multiple values from a single attribute?

I'm not sure what the code in usn_start() is doing. Is nsds5ReplicatedAttributeList multi-valued? If so, each char * in char **old_frac_attrs will be a full attribute list description "(objectclass=*) $ EXCLUDE ...". Then I'm not sure what the code that adds the entryusn attribute to the default excluded list is doing - will it add entryusn to only the first value of nsds5ReplicatedAttributeList? And wipe out the other values? I think that's what will happen, since there is a break in the for loop if token == NULL - it will skip old_frac_attrs[1] etc. Also the call to charray_add - the token argument is not a malloced string, so that will cause problems when slapi_ch_array_free() is called.

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

--
389-devel mailing list
389-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/fedora-directory-devel

[Index of Archives]     [Fedora Directory Announce]     [Fedora Users]     [Older Fedora Users Mail]     [Fedora Advisory Board]     [Fedora Security]     [Fedora Devel Java]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Mentors]     [Fedora Package Review]     [Fedora Art]     [Fedora Music]     [Fedora Packaging]     [CentOS]     [Fedora SELinux]     [Big List of Linux Books]     [KDE Users]     [Fedora Art]     [Fedora Docs]

  Powered by Linux