I think all the files referring DB_BUFFER_SMALL has the define...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.patchLooks 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
I guess not. :) I'm going to change the type...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?
Calling cursor->c_get with DB_SET and DB_GET_BOTH were redundant. Since this is delete, we have both the key and value. All we need to do should be setting the cursor at the position and delete the key-value pair.idl_new_delete_key - why the changes there?
It was found by valgrind reporting a memory leak. The DBT key has DB_DBT_MALLOC flag set underneath. Once cursor->c_get is called with DB_SET, the memory is allocated and filled with the key. (Freeing the key->data fixes the leak. But I believe we don't have to call the cursor->c_get twice here.)DB_GET_BOTH
Move the cursor to the specified key/data pair of the database. The cursor is positioned to a key/data pair if both the key and data match the values provided on the key and data parameters.
In all other ways, this flag is identical to the DB_SET flag.
Well, I thought slapi_mod_done wants to do 0 pudding the whole Slapi_Mods not just the first pointer... Do you think the sizeof(smod) is intentional?modutil.c line 632 - why the change there?
Oops. My mistake. I wanted to add @nspr_inc@to dbscan_bin_CPPFLAGS. I mistakenly assumed NSPR is not linked. :pI 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?