[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1469520 - /subversion/trunk/subversion/libsvn_fs_fs/tree.c

From: Blair Zajac <blair_at_orcaware.com>
Date: Mon, 13 May 2013 23:05:46 -0700

On 4/18/13 11:53 AM, Philip Martin wrote:
> blair_at_apache.org writes:
>
>> Author: blair
>> Date: Thu Apr 18 18:41:55 2013
>> New Revision: 1469520
>>
>> URL: http://svn.apache.org/r1469520
>> Log:
>> open_path(): silence compiler warning.
>>
>> subversion/libsvn_fs_fs/tree.c: In function 'open_path':
>> subversion/libsvn_fs_fs/tree.c:916: warning: 'directory' may be used uninitialized in this function
>>
>> * subversion/libsvn_fs_fs/tree.c
>> (open_path):
>> Set directory to NULL to silence a compiler warning.
>>
>> Modified:
>> subversion/trunk/subversion/libsvn_fs_fs/tree.c
>>
>> Modified: subversion/trunk/subversion/libsvn_fs_fs/tree.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tree.c?rev=1469520&r1=1469519&r2=1469520&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_fs_fs/tree.c (original)
>> +++ subversion/trunk/subversion/libsvn_fs_fs/tree.c Thu Apr 18 18:41:55 2013
>> @@ -913,7 +913,7 @@ open_path(parent_path_t **parent_path_p,
>> 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;
>> + const char *directory = NULL;
>> assert(svn_fs__is_canonical_abspath(path));
>> if (flags & open_path_node_only)
>> {
>>
>>
>
> In the past we have not been adding these sorts of initialisations. A
> build without warnings is nice but unnecessary initialisation can reduce
> the effectiveness of memory checking tools. In this particular case the
> code is:

I just saw this now when looking through STATUS.

I would say that we have been keeping our build warning free though, if it takes
a initialization.

>
> if (here)
> {
> path_so_far = directory;
> rest = path + strlen(directory) + 1;
> }
>
> Passing NULL to strlen is not guaranteed to work but will work on some
> platforms. Without the initialisation a memory checking tool like
> valgrind will trigger if strlen were to access directory. The
> initialisation means there is no longer a guarantee that valgrind will
> trigger.

I would say though that I would prefer keeping the runtime in better shape, say
reading from NULL and getting a core dumo, than random memory location.

I did assume that the code was good though and just did this to silence a warning.

Blair
Received on 2013-05-14 08:06:27 CEST

This is an archived mail posted to the Subversion Dev mailing list.