Philip Martin wrote:
> Daniel Shahaf <danielsh_at_elego.de> writes:
>> What would a better fix be?
>
> It's hard to say which is probably why these warnings have not been
> addressed so far. The warning is a 'maybe':
>
> ../src/subversion/libsvn_fs_fs/tree.c:929:27: warning: 'directory' may
> be used uninitialized in this function [-Wmaybe-uninitialized]
>
> so what will make it go away probably depends on various compiler
> details. A better compiler optimiser might make the warning go away.
Agreed.
>> The following is a first sketch, it should resolve the compiler warning
>> but won't solve the actual semantic bug (if there is one) here.
>
> I believe it's a false positive and there is no bug, other than the
> warning, as 'directory' has to be non-NULL if 'here' is
> non-NULL.
Agreed.
I don't think Daniel's assertion made things clearer. Here's my code rearrangement suggestion, moving the 'directory' variable into a smaller scope and avoiding it ever being uninitialized:
[[[
Index: subversion/libsvn_fs_fs/tree.c
===================================================================
--- subversion/libsvn_fs_fs/tree.c (revision 1469858)
+++ subversion/libsvn_fs_fs/tree.c (working copy)
@@ -910,28 +910,28 @@ open_path(parent_path_t **parent_path_p,
apr_pool_t *iterpool = svn_pool_create(pool);
/* callers often traverse the tree in some path-based order. That means
a sibling of PATH has been presently accessed. Try to start the lookup
directly at the parent node, if the caller did not requested the full
parent chain. */
- const char *directory = NULL;
assert(svn_fs__is_canonical_abspath(path));
if (flags & open_path_node_only)
{
- directory = svn_dirent_dirname(path, pool);
+ const char *directory = svn_dirent_dirname(path, pool);
+
if (directory[1] != 0) /* root nodes are covered anyway */
SVN_ERR(dag_node_cache_get(&here, root, directory, TRUE, pool));
+ if (here)
+ {
+ path_so_far = directory;
+ rest = path + strlen(directory) + 1;
+ }
}
- /* did the shortcut work? */
- if (here)
- {
- path_so_far = directory;
- rest = path + strlen(directory) + 1;
- }
- else
+ /* If we didn't take the shortcut ... */
+ if (! here)
{
/* Make a parent_path item for the root node, using its own current
copy id. */
SVN_ERR(root_node(&here, root, pool));
rest = path + 1; /* skip the leading '/', it saves in iteration */
}
]]]
- Julian
Received on 2013-04-19 17:42:58 CEST