On 01/19/2010 01:50 PM, Rich Megginson wrote:
Noriko Hosoi wrote:
> Allow modrdn to move subtree and rename non-leaf node
>
> This patch includes
> - replacing the entrydn index with the entryrdn index
> - replacing a full DN in each entry in the DB with an RDN
> - extending Slapi_Entry, entry2str, and str2entry to absorb the
> changes made on the entry
> - adding DN/RDN helper functions
> - adding DN cache
> - adding a utility and a migration script to convert the DN format
> database to the RDN format
> - extending a database dump utility dbscan to support the entryrdn
>
> In addition to the above, compile warnings and memory leaks found
> in testing the new feature are fixed.
>
> For more details, see the feature design document at:
>
>
http://directory.fedoraproject.org/wiki/Subtree_Rename
>
> The patch is too big to attach to the email. It is located here:
>
>
http://nhosoi.fedorapeople.org/0001-Allow-modrdn-to-move-subtree-and-rena...
>
>
Looks good. Just a few questions.
Note that DB_BUFFER_SMALL was new for db 4.3 - I suggest using the code
in cl5_clcache.c that does this:
/* newer bdb uses DB_BUFFER_SMALL instead of ENOMEM as the
error return if the given buffer in which to load a
key or value is too small - if it is not defined, define
it here to ENOMEM
*/
#ifndef DB_BUFFER_SMALL
#define DB_BUFFER_SMALL ENOMEM
#endif
in id2entry.c line 295 - you have const char *dn - then you have to cast
away the const almost everywhere it is used - just make it a char *?
does it need to be const?
idl_new_delete_key - why the changes there?
modutil.c line 632 - why the change there?
I have one other question to tack on from my review.
In Makefile.am, you have:
-dbscan_bin_CPPFLAGS = @db_inc@ $(AM_CPPFLAGS)
-dbscan_bin_LDADD = $(NSPR_LINK) $(DB_LINK)
+dbscan_bin_CPPFLAGS = @db_inc@ @nspr_inc@ $(AM_CPPFLAGS)
+dbscan_bin_LDADD = $(NSPR_LINK) $(DB_LINK) $(NSPR_LINK)
Why is $NSPR_LINK listed twice?
> Thanks,
> --noriko
>
> ------------------------------------------------------------------------
>
> --
> 389-devel mailing list
> 389-devel(a)lists.fedoraproject.org
>
https://admin.fedoraproject.org/mailman/listinfo/389-devel
>
>
--
389-devel mailing list
389-devel(a)lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/389-devel