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-rename-non-leaf-nod.patch >> >> > 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@xxxxxxxxxxxxxxxxxxxxxxx >> https://admin.fedoraproject.org/mailman/listinfo/389-devel >> >> > -- > 389-devel mailing list > 389-devel@xxxxxxxxxxxxxxxxxxxxxxx > https://admin.fedoraproject.org/mailman/listinfo/389-devel > -- 389-devel mailing list 389-devel@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/389-devel
