[389-devel] Please review: Allow modrdn to move subtree and rename non-leaf node

[Nathan Kinder]

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 at lists.fedoraproject.org
>> https://admin.fedoraproject.org/mailman/listinfo/389-devel
>>
>>      
> --
> 389-devel mailing list
> 389-devel at lists.fedoraproject.org
> https://admin.fedoraproject.org/mailman/listinfo/389-devel
>