You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@wandisco.com> on 2012/07/10 13:15:06 UTC

Re: svn commit: r1359574 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

stefan2@apache.org writes:

> Author: stefan2
> Date: Tue Jul 10 10:19:42 2012
> New Revision: 1359574

> +        else
> +          right_size += APR_ARRAY_IDX(revprops->sizes, right--, apr_off_t);
> +                      + SVN_INT64_BUFFER_SIZE;

../src/subversion/libsvn_fs_fs/fs_fs.c: In function ‘write_packed_revprop’:
../src/subversion/libsvn_fs_fs/fs_fs.c:4018: warning: statement with no effect

Also, putting right-- in a macro like APR_ARRAY_IDX relies on the macro
not expanding to use the parameter twice.

-- 
Philip

Re: svn commit: r1359574 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Tue, Jul 10, 2012 at 6:34 PM, Philip Martin
<ph...@wandisco.com>wrote:

> Stefan Fuhrmann <st...@wandisco.com> writes:
>
> > On Tue, Jul 10, 2012 at 1:15 PM, Philip Martin
> > <ph...@wandisco.com>wrote:
> >
> >> stefan2@apache.org writes:
> >>
> >> > Author: stefan2
> >> > Date: Tue Jul 10 10:19:42 2012
> >> > New Revision: 1359574
> >>
> >> > +        else
> >> > +          right_size += APR_ARRAY_IDX(revprops->sizes, right--,
> >> apr_off_t);
> >> > +                      + SVN_INT64_BUFFER_SIZE;
> >>
> >> ../src/subversion/libsvn_fs_fs/fs_fs.c: In function
> ‘write_packed_revprop’:
> >> ../src/subversion/libsvn_fs_fs/fs_fs.c:4018: warning: statement with no
> >> effect
> >>
> >
> > D'oh!
> >
> >
> >> Also, putting right-- in a macro like APR_ARRAY_IDX relies on the macro
> >> not expanding to use the parameter twice.
> >>
> >
> > Thanks for the review!
> > Fixed in r1359753.
>
> I'm seeing fs-pack-test 6 failing.
>
> $ ./fs-pack-test 6
> ../src/subversion/tests/svn_test_main.c:275: (apr_err=200006)
> svn_tests: E200006: Test crashed (run in debugger with '--allow-segfaults')
> FAIL:  lt-fs-pack-test 6: get/set large packed revprops in FSFS
>

*Sigh* Fixed in r1359771.
Note to self: always, *always* run tests before comitting ...

-- Stefan^2.

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download

Re: svn commit: r1359574 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

Posted by Philip Martin <ph...@wandisco.com>.
Stefan Fuhrmann <st...@wandisco.com> writes:

> On Tue, Jul 10, 2012 at 1:15 PM, Philip Martin
> <ph...@wandisco.com>wrote:
>
>> stefan2@apache.org writes:
>>
>> > Author: stefan2
>> > Date: Tue Jul 10 10:19:42 2012
>> > New Revision: 1359574
>>
>> > +        else
>> > +          right_size += APR_ARRAY_IDX(revprops->sizes, right--,
>> apr_off_t);
>> > +                      + SVN_INT64_BUFFER_SIZE;
>>
>> ../src/subversion/libsvn_fs_fs/fs_fs.c: In function ‘write_packed_revprop’:
>> ../src/subversion/libsvn_fs_fs/fs_fs.c:4018: warning: statement with no
>> effect
>>
>
> D'oh!
>
>
>> Also, putting right-- in a macro like APR_ARRAY_IDX relies on the macro
>> not expanding to use the parameter twice.
>>
>
> Thanks for the review!
> Fixed in r1359753.

I'm seeing fs-pack-test 6 failing.

$ ./fs-pack-test 6
../src/subversion/tests/svn_test_main.c:275: (apr_err=200006)
svn_tests: E200006: Test crashed (run in debugger with '--allow-segfaults')
FAIL:  lt-fs-pack-test 6: get/set large packed revprops in FSFS

valgrind reports:

==18988== Invalid read of size 8
==18988==    at 0x5255DA5: write_packed_revprop (fs_fs.c:4013)
==18988==    by 0x5256441: set_revision_proplist (fs_fs.c:4131)
==18988==    by 0x5263011: change_rev_prop_body (fs_fs.c:8996)
==18988==    by 0x524DD47: with_some_lock_file (fs_fs.c:614)
==18988==    by 0x524DE1D: svn_fs_fs__with_write_lock (fs_fs.c:632)
==18988==    by 0x52630B1: svn_fs_fs__change_rev_prop (fs_fs.c:9017)
==18988==    by 0x503ABE4: svn_fs_change_rev_prop2 (fs-loader.c:1209)
==18988==    by 0x503AC3D: svn_fs_change_rev_prop (fs-loader.c:1218)
==18988==    by 0x4036C2: get_set_huge_revprop_packed_fs (fs-pack-test.c:561)
==18988==    by 0x4E2EBFF: do_test_num (svn_test_main.c:265)
==18988==    by 0x4E2F602: main (svn_test_main.c:525)
==18988==  Address 0x9892ca8 is 0 bytes after a block of size 24 alloc'd
==18988==    at 0x4C244E8: malloc (vg_replace_malloc.c:236)
==18988==    by 0x5B5D6D0: pool_alloc (apr_pools.c:1463)
==18988==    by 0x5B5D7A5: apr_pcalloc_debug (apr_pools.c:1520)
==18988==    by 0x5B56C66: make_array_core (apr_tables.c:66)
==18988==    by 0x5B56F49: apr_array_make (apr_tables.c:89)
==18988==    by 0x525465C: parse_packed_revprops (fs_fs.c:3509)
==18988==    by 0x5254C58: read_pack_revprop (fs_fs.c:3627)
==18988==    by 0x5255A4D: write_packed_revprop (fs_fs.c:3968)
==18988==    by 0x5256441: set_revision_proplist (fs_fs.c:4131)
==18988==    by 0x5263011: change_rev_prop_body (fs_fs.c:8996)
==18988==    by 0x524DD47: with_some_lock_file (fs_fs.c:614)
==18988==    by 0x524DE1D: svn_fs_fs__with_write_lock (fs_fs.c:632)

followed by

==18988== Invalid read of size 8
==18988==    at 0x5255D7F: write_packed_revprop (fs_fs.c:4012)

==18988== Invalid read of size 8
==18988==    at 0x5255DFA: write_packed_revprop (fs_fs.c:4021)

==18988== Invalid read of size 8
==18988==    at 0x5255DCD: write_packed_revprop (fs_fs.c:4015)

==18988== Conditional jump or move depends on uninitialised value(s)
==18988==    at 0x5255DAF: write_packed_revprop (fs_fs.c:4012)

-- 
Cerified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download

Re: svn commit: r1359574 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Tue, Jul 10, 2012 at 1:15 PM, Philip Martin
<ph...@wandisco.com>wrote:

> stefan2@apache.org writes:
>
> > Author: stefan2
> > Date: Tue Jul 10 10:19:42 2012
> > New Revision: 1359574
>
> > +        else
> > +          right_size += APR_ARRAY_IDX(revprops->sizes, right--,
> apr_off_t);
> > +                      + SVN_INT64_BUFFER_SIZE;
>
> ../src/subversion/libsvn_fs_fs/fs_fs.c: In function ‘write_packed_revprop’:
> ../src/subversion/libsvn_fs_fs/fs_fs.c:4018: warning: statement with no
> effect
>

D'oh!


> Also, putting right-- in a macro like APR_ARRAY_IDX relies on the macro
> not expanding to use the parameter twice.
>

Thanks for the review!
Fixed in r1359753.

-- Stefan^2.

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download