On Thu, Jul 25, 2013 at 2:37 PM, Ben Reser <ben_at_reser.org> wrote:
> Well looks like we've got quite a bit of cleanup to do then...
>
> [breser_at_fmri subversion]$ ack 'svn_hash_sets.*apr_' --noheading --nobreak
> svn/notify.c:95: svn_hash_sets(hash,
> apr_pstrdup(nb->conflict_stats->stats_pool, path), "");
> svn/cl-conflicts.c:320: svn_hash_sets(att_hash, "revision",
> apr_ltoa(pool, version->peg_rev));
> bindings/swig/perl/libsvn_swig_perl/swigutil_pl.c:190:
> svn_hash_sets(hash, apr_pstrmemdup(pool, key, retlen), val);
> bindings/swig/ruby/libsvn_swig_ruby/swigutil_rb.c:1529:
> svn_hash_sets(data->apr_hash, apr_pstrdup(data->pool,
> StringValuePtr(key)),
> libsvn_client/prop_commands.c:702: svn_hash_sets(b->props,
> apr_pstrdup(b->pool, local_abspath),
> libsvn_client/commit_util.c:931: svn_hash_sets(danglers,
> apr_pstrdup(result_pool, parent_abspath),
> libsvn_client/locking_commands.c:452: svn_hash_sets(path_tokens,
> path, apr_pstrdup(pool, lock->token));
> libsvn_client/copy_foreign.c:168: svn_hash_sets(db->properties,
> apr_pstrdup(db->pool, name),
> libsvn_client/copy_foreign.c:314: svn_hash_sets(fb->properties,
> apr_pstrdup(fb->pool, name),
> libsvn_client/merge.c:11087: svn_hash_sets(new_catalog,
> apr_pstrdup(scratch_pool, source_path),
> libsvn_client/commit.c:380: svn_hash_sets(wc_items,
> apr_pstrdup(scratch_pool, wcroot_abspath),
> libsvn_subr/mergeinfo.c:1877: svn_hash_sets(*mergeinfo,
> apr_pstrdup(result_pool, path),
> libsvn_subr/mergeinfo.c:2009:
> svn_hash_sets(new_mergeinfo_catalog, apr_pstrdup(pool, key),
> libsvn_subr/mergeinfo.c:2496:
> svn_hash_sets(*adjusted_mergeinfo, apr_pstrdup(result_pool, path),
> libsvn_subr/subst.c:1521: svn_hash_sets(copy,
> apr_pstrdup(result_pool, key),
> libsvn_subr/types.c:328:
> svn_hash_sets(new_entry->changed_paths2, apr_pstrdup(pool, key),
> libsvn_subr/dso.c:95: svn_hash_sets(dso_cache,
> apr_pstrdup(dso_pool, fname), NOT_THERE);
> libsvn_subr/dso.c:101: svn_hash_sets(dso_cache,
> apr_pstrdup(dso_pool, fname), *dso);
> libsvn_repos/log.c:1875: svn_hash_sets(mergeinfo,
> apr_pstrdup(processed_pool, path), ranges);
> libsvn_repos/fs-wrap.c:619: svn_hash_sets(b->locks,
> apr_pstrdup(hash_pool, lock->path),
> libsvn_repos/hooks.c:384: svn_hash_sets(bo->hooks_env,
> apr_pstrdup(result_pool, bo->section),
> libsvn_repos/hooks.c:387: svn_hash_sets(hook_env,
> apr_pstrdup(result_pool, name),
> libsvn_fs_fs/tree.c:3847: svn_hash_sets(result_catalog,
> apr_pstrdup(result_pool, kid_path),
> libsvn_fs_base/tree.c:5302: svn_hash_sets(result_catalog,
> apr_pstrdup(result_pool, path),
> libsvn_wc/diff_editor.c:1552: svn_hash_sets(pb->compared,
> apr_pstrdup(pb->pool, db->name), "");
> libsvn_wc/diff_editor.c:1629: svn_hash_sets(pb->compared,
> apr_pstrdup(pb->pool, db->name), "");
> libsvn_wc/diff_editor.c:1837: svn_hash_sets(pb->compared,
> apr_pstrdup(pb->pool, fb->name), "");
> libsvn_wc/diff_editor.c:1910: svn_hash_sets(pb->compared,
> apr_pstrdup(pb->pool, fb->name), "");
> libsvn_wc/status.c:1562: svn_hash_sets(stat_hash, apr_pstrdup(hash_pool, path),
> libsvn_wc/status.c:1645: svn_hash_sets(statushash,
> apr_pstrdup(pool, local_abspath), statstruct);
> libsvn_wc/wc_db.c:8647: svn_hash_sets(nodes,
> apr_pstrdup(result_pool, name), child);
> libsvn_wc/wc_db.c:8730: svn_hash_sets(conflicts,
> apr_pstrdup(result_pool, name), "");
> libsvn_wc/wc_db.c:8972: svn_hash_sets(*nodes,
> apr_pstrdup(result_pool, name), child);
> libsvn_wc/workqueue.c:1658: svn_hash_sets(wqb->record_map,
> apr_pstrdup(wqb->result_pool, local_abspath),
> libsvn_wc/upgrade.c:205: svn_hash_sets(*all_wcprops,
> apr_pstrdup(result_pool, name), wcprops);
> libsvn_wc/update_editor.c:1857:
> svn_hash_sets(pb->deletion_conflicts, apr_pstrdup(pb->pool, base),
> libsvn_wc/update_editor.c:3169:
> svn_hash_sets(pb->not_present_files, apr_pstrdup(pb->pool, fb->name),
> libsvn_wc/update_editor.c:3277:
> svn_hash_sets(pb->not_present_files, apr_pstrdup(pb->pool, fb->name),
> libsvn_wc/update_editor.c:3382:
> svn_hash_sets(pb->not_present_files, apr_pstrdup(pb->pool, fb->name),
> libsvn_wc/entries.c:1923: svn_hash_sets(tree_conflicts,
> apr_pstrdup(result_pool, key),
> libsvn_wc/wc_db_wcroot.c:869: svn_hash_sets(db->dir_data,
> apr_pstrdup(db->state_pool, parent_dir),
> libsvn_wc/wc_db_wcroot.c:908: svn_hash_sets(db->dir_data,
> svn__apr_hash_index_key(hi), NULL);
> svnrdump/dump_editor.c:672: svn_hash_sets(pb->deleted_entries,
> apr_pstrdup(pb->eb->pool, path), pb);
> svnrdump/dump_editor.c:903: svn_hash_sets(db->deleted_props,
> apr_pstrdup(db->pool, name), "");
> svnrdump/dump_editor.c:931: svn_hash_sets(fb->deleted_props,
> apr_pstrdup(fb->pool, name), "");
> libsvn_delta/compat.c:1050: svn_hash_sets(changes,
> apr_pstrdup(result_pool, relpath), change);
The preceding may have picked up some false positives since it's ok to
dup the memory in the value variable since it's only evaluated once in
the macro (but probably not since those are usually wrapped on another
line).
The list below have the same problem as well, they didn't show up on
my initial search because they are either split across multiple lines
or do something other than call an apr call. Some of them are far
worse than the simple duplication of memory since they actually do
work to produce the value. I didn't investigate the cases where there
are macros used within the macros. Which happens quite often, but I'm
pretty sure all of those are just raw strings.
There are some macros that wrap svn_hash_sets which may be bad based
on the user of the macro (see libsvn_delta/editor.c:113), at current
this particular one looks ok but it's liable to duplicate unless every
future user is knowledgable about this.
Rather than trying to fix all of this I think we should just convert
svn_hash_sets from a macro to a function that's properly flagged so
that it can be inlined.
libsvn_client/prop_commands.c
620: svn_hash_sets(new_iprop->prop_hash,
633: svn_hash_sets(props,
libsvn_client/mergeinfo.c
408: svn_hash_sets(*mergeinfo_cat,
1610: svn_hash_sets(full_path_mergeinfo,
libsvn_client/commit_util.c
223: svn_hash_sets(committables->by_repository,
libsvn_client/switch.c
72: svn_hash_sets(conflicted_paths,
libsvn_client/iprops.c
220: svn_hash_sets(*wcroot_iprops,
libsvn_client/merge.c
1383: svn_hash_sets(parent_baton->new_tree_conflicts,
5223: svn_hash_sets(result_catalog,
8032: svn_hash_sets(log_baton->operative_children,
10744: svn_hash_sets(log_baton->unmerged_catalog,
11159: svn_hash_sets(new_catalog,
libsvn_client/add.c
565: svn_hash_sets(autoprops_baton->autoprops,
569: svn_hash_sets(pattern_hash,
libsvn_client/update.c
177: svn_hash_sets(conflicted_paths,
libsvn_client/list.c
140: svn_hash_sets(externals,
libsvn_subr/mergeinfo.c
2201: svn_hash_sets(*out_catalog,
2229: svn_hash_sets(*out_mergeinfo,
2405: svn_hash_sets(*filtered_cat,
2450: svn_hash_sets(*filtered_mergeinfo,
libsvn_subr/auth.c
271: svn_hash_sets(auth_baton->creds_cache,
410: svn_hash_sets(auth_baton->creds_cache,
libsvn_ra_svn/client.c
1128: svn_hash_sets(new_iprop->prop_hash,
1363: svn_hash_sets(*catalog, path[0] == '/' ? path + 1
:path, for_path); // this one isn't so bad
libsvn_repos/rev_hunt.c
1209: svn_hash_sets(duplicate_path_revs,
libsvn_fs_base/tree.c
5034: svn_hash_sets(args->result_catalog,
5057: svn_hash_sets(args->children_atop_mergeinfo_trees,
libsvn_wc/diff_editor.c
1613: svn_hash_sets(pb->compared,
1892: svn_hash_sets(pb->compared,
libsvn_wc/wc_db.c
1229: svn_hash_sets(*children,
3580: svn_hash_sets(*externals,
9553: svn_hash_sets(*values,
14885: svn_hash_sets(*lock_tokens,
libsvn_wc/info.c
437: svn_hash_sets(fe_baton->tree_conflicts,
libsvn_wc/upgrade.c
701: svn_hash_sets(*conflicts,
1770: svn_hash_sets(repos_cache,
2264: svn_hash_sets(repos_cache,
libsvn_wc/update_editor.c
278: svn_hash_sets(eb->skipped_trees,
4937: svn_hash_sets(eb->dir_dirents,
4993: svn_hash_sets(eb->dir_dirents,
libsvn_wc/wc_db_wcroot.c
841: svn_hash_sets(db->dir_data,
908: svn_hash_sets(db->dir_data, svn__apr_hash_index_key(hi), NULL);
libsvn_wc/wc_db_update_move.c
2246: svn_hash_sets(src_done,
libsvn_wc/conflicts.c
894: svn_hash_sets(*conflicted_prop_names,
svndumpfilter/svndumpfilter.c
564: svn_hash_sets(pb->dropped_nodes,
842: svn_hash_sets(rb->props,
svnrdump/dump_editor.c
899: svn_hash_sets(db->props,
927: svn_hash_sets(fb->props,
svnrdump/load_editor.c
861: svn_hash_sets(rb->revprop_table,
libsvn_ra_serf/xml.c
614: svn_hash_sets(scan->attrs,
libsvn_ra_serf/mergeinfo.c
112: svn_hash_sets(mergeinfo_ctx->result_catalog,
libsvn_ra_serf/inherited_props.c
183: svn_hash_sets(iprops_ctx->curr_iprop->prop_hash,
libsvn_ra_serf/commit.c
1605: svn_hash_sets(dir->commit->deleted_entries,
2314: svn_hash_sets(ctx->revprop_table,
2317: svn_hash_sets(ctx->revprop_table,
libsvn_ra_serf/update.c
2646: svn_hash_sets(report->lock_path_tokens,
libsvn_fs_util/fs-util.c
217: svn_hash_sets(*output,
libsvn_delta/editor.c
116: svn_hash_sets((editor)->completed_nodes, \
libsvn_delta/compat.c
275: svn_hash_sets(change->props,
Received on 2013-07-26 00:27:36 CEST