Re: [PATCH] txn_body_dir_entries on success never sets *(baton)->table_p null(leverage the usage on this fact)

Kamesh Jayachandran wrote:
> Can someone look into this?

Due to its similarity, I've attached this patch to issue 2637.

-Hyrum

> With regards
> Kamesh Jayachandran
> Kamesh Jayachandran wrote:
>> Hi All,
>> Find the attached patch and log.
>>
>> With regards
>> Kamesh Jayachandran
>> ------------------------------------------------------------------------
>>
>> [[[
>> Patch by: Kamesh Jayachandran <kamesh@collab.net>
>>
>> 'txn_body_dir_entries' on success never sets the '*(baton)->entries'
>> to null, leverage the same in its usage.
>>
>> * subversion/libsvn_fs_base/tree.c
>> (txn_body_dir_entries):
>> Add a DOCSTRING that says on success it never sets
>> *(baton)->entries to NULL.
>> (base_dir_entries):
>> Never check for *(args.table_p), on success of txn_body_dir_entries
>> it will not be NULL.
>> ]]]
>>
>> ------------------------------------------------------------------------
>>
>> Index: subversion/libsvn_fs_base/tree.c
>> ===================================================================
>> --- subversion/libsvn_fs_base/tree.c (revision 21821)
>> +++ subversion/libsvn_fs_base/tree.c (working copy)
>> @@ -1426,6 +1426,7 @@
>> };
>>
>>
>> +/* *(BATON)->entries will never be NULL on successful return */
>> static svn_error_t *
>> txn_body_dir_entries(void *baton,
>> trail_t *trail)
>> @@ -1454,6 +1455,7 @@
>> struct dir_entries_args args;
>> apr_hash_t *table;
>> svn_fs_t *fs = root->fs;
>> + apr_hash_index_t *hi;
>>
>> args.table_p = &table;
>> args.root = root;
>> @@ -1462,29 +1464,21 @@
>> pool));
>>
>> /* Add in the kind data. */
>> - if (table)
>> + for (hi = apr_hash_first(pool, table); hi; hi = apr_hash_next(hi))
>> {
>> - apr_hash_index_t *hi;
>> - for (hi = apr_hash_first(pool, table); hi; hi = apr_hash_next(hi))
>> - {
>> - svn_fs_dirent_t *entry;
>> - struct node_kind_args nk_args;
>> - void *val;
>> + svn_fs_dirent_t *entry;
>> + struct node_kind_args nk_args;
>> + void *val;
>>
>> - /* KEY will be the entry name in ancestor (about which we
>> - simple don't care), VAL the dirent. */
>> - apr_hash_this(hi, NULL, NULL, &val);
>> - entry = val;
>> - nk_args.id = entry->id;
>> - SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_node_kind,
>> &nk_args,
>> - pool));
>> - entry->kind = nk_args.kind;
>> - }
>> + /* KEY will be the entry name in ancestor (about which we
>> + simple don't care), VAL the dirent. */
>> + apr_hash_this(hi, NULL, NULL, &val);
>> + entry = val;
>> + nk_args.id = entry->id;
>> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_node_kind, &nk_args,
>> + pool));
>> + entry->kind = nk_args.kind;
>> }
>> - else
>> - {
>> - table = apr_hash_make(pool);
>> - }
>>
>> *table_p = table;
>> return SVN_NO_ERROR;