You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by iv...@apache.org on 2014/03/13 13:53:40 UTC
svn commit: r1577144 - /subversion/trunk/subversion/libsvn_ra/compat.c
Author: ivan
Date: Thu Mar 13 12:53:40 2014
New Revision: 1577144
URL: http://svn.apache.org/r1577144
Log:
Fix problem exposed by r1577079.
* subversion/libsvn_ra/compat.c
(svn_ra__locations_from_log): Make a copy of constant apr_array_header_t
argument before sorting it.
Modified:
subversion/trunk/subversion/libsvn_ra/compat.c
Modified: subversion/trunk/subversion/libsvn_ra/compat.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra/compat.c?rev=1577144&r1=1577143&r2=1577144&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra/compat.c (original)
+++ subversion/trunk/subversion/libsvn_ra/compat.c Thu Mar 13 12:53:40 2014
@@ -316,6 +316,7 @@ svn_ra__locations_from_log(svn_ra_sessio
svn_revnum_t youngest_requested, oldest_requested, youngest, oldest;
svn_node_kind_t kind;
const char *fs_path;
+ apr_array_header_t *sorted_location_revisions;
/* Fetch the absolute FS path associated with PATH. */
SVN_ERR(get_fs_path(&fs_path, session, path, pool));
@@ -337,10 +338,11 @@ svn_ra__locations_from_log(svn_ra_sessio
/* Figure out the youngest and oldest revs (amongst the set of
requested revisions + the peg revision) so we can avoid
unnecessary log parsing. */
- svn_sort__array(location_revisions, compare_revisions);
- oldest_requested = APR_ARRAY_IDX(location_revisions, 0, svn_revnum_t);
- youngest_requested = APR_ARRAY_IDX(location_revisions,
- location_revisions->nelts - 1,
+ sorted_location_revisions = apr_array_copy(pool, location_revisions);
+ svn_sort__array(sorted_location_revisions, compare_revisions);
+ oldest_requested = APR_ARRAY_IDX(sorted_location_revisions, 0, svn_revnum_t);
+ youngest_requested = APR_ARRAY_IDX(sorted_location_revisions,
+ sorted_location_revisions->nelts - 1,
svn_revnum_t);
youngest = peg_revision;
youngest = (oldest_requested > youngest) ? oldest_requested : youngest;
@@ -352,7 +354,7 @@ svn_ra__locations_from_log(svn_ra_sessio
/* Populate most of our log receiver baton structure. */
lrb.kind = kind;
lrb.last_path = fs_path;
- lrb.location_revisions = apr_array_copy(pool, location_revisions);
+ lrb.location_revisions = apr_array_copy(pool, sorted_location_revisions);
lrb.peg_revision = peg_revision;
lrb.peg_path = NULL;
lrb.locations = locations;
@@ -379,9 +381,9 @@ svn_ra__locations_from_log(svn_ra_sessio
if (lrb.last_path)
{
int i;
- for (i = 0; i < location_revisions->nelts; i++)
+ for (i = 0; i < sorted_location_revisions->nelts; i++)
{
- svn_revnum_t rev = APR_ARRAY_IDX(location_revisions, i,
+ svn_revnum_t rev = APR_ARRAY_IDX(sorted_location_revisions, i,
svn_revnum_t);
if (! apr_hash_get(locations, &rev, sizeof(rev)))
apr_hash_set(locations, apr_pmemdup(pool, &rev, sizeof(rev)),
Re: svn commit: r1577144 - /subversion/trunk/subversion/libsvn_ra/compat.c
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 13 March 2014 17:13, Branko Čibej <br...@wandisco.com> wrote:
> On 13.03.2014 13:53, ivan@apache.org wrote:
>
> Author: ivan
> Date: Thu Mar 13 12:53:40 2014
> New Revision: 1577144
>
> URL: http://svn.apache.org/r1577144
> Log:
> Fix problem exposed by r1577079.
>
> * subversion/libsvn_ra/compat.c
> (svn_ra__locations_from_log): Make a copy of constant apr_array_header_t
> argument before sorting it.
>
>
> [...]
>
>
> @@ -337,10 +338,11 @@ svn_ra__locations_from_log(svn_ra_sessio
> /* Figure out the youngest and oldest revs (amongst the set of
> requested revisions + the peg revision) so we can avoid
> unnecessary log parsing. */
> - svn_sort__array(location_revisions, compare_revisions);
> - oldest_requested = APR_ARRAY_IDX(location_revisions, 0, svn_revnum_t);
> - youngest_requested = APR_ARRAY_IDX(location_revisions,
> - location_revisions->nelts - 1,
> + sorted_location_revisions = apr_array_copy(pool, location_revisions);
> + svn_sort__array(sorted_location_revisions, compare_revisions);
> + oldest_requested = APR_ARRAY_IDX(sorted_location_revisions, 0,
> svn_revnum_t);
> + youngest_requested = APR_ARRAY_IDX(sorted_location_revisions,
> + sorted_location_revisions->nelts - 1,
> svn_revnum_t);
> youngest = peg_revision;
> youngest = (oldest_requested > youngest) ? oldest_requested : youngest;
> @@ -352,7 +354,7 @@ svn_ra__locations_from_log(svn_ra_sessio
> /* Populate most of our log receiver baton structure. */
> lrb.kind = kind;
> lrb.last_path = fs_path;
> - lrb.location_revisions = apr_array_copy(pool, location_revisions);
> + lrb.location_revisions = apr_array_copy(pool, sorted_location_revisions);
>
>
> So now you've copied the array twice?
>
Yes, I do. Because lrb.locations_revisions will be modified by
svn_ra_get_log3() callback, while we need unmodified and sorted
location revisions if the received log information did not cover any
of the requested revisions:
[[[
if (! lrb.peg_path)
lrb.peg_path = lrb.last_path;
if (lrb.last_path)
{
int i;
for (i = 0; i < sorted_location_revisions->nelts; i++)
{
svn_revnum_t rev = APR_ARRAY_IDX(sorted_location_revisions, i,
svn_revnum_t);
if (! apr_hash_get(locations, &rev, sizeof(rev)))
apr_hash_set(locations, apr_pmemdup(pool, &rev, sizeof(rev)),
sizeof(rev), apr_pstrdup(pool, lrb.last_path));
}
}
]]]
So I decided to just copy it twice given that this is fallback code
for older servers and don't covered by our test suite.
--
Ivan Zhakov
Re: svn commit: r1577144 - /subversion/trunk/subversion/libsvn_ra/compat.c
Posted by Branko Čibej <br...@wandisco.com>.
On 13.03.2014 13:53, ivan@apache.org wrote:
> Author: ivan
> Date: Thu Mar 13 12:53:40 2014
> New Revision: 1577144
>
> URL: http://svn.apache.org/r1577144
> Log:
> Fix problem exposed by r1577079.
>
> * subversion/libsvn_ra/compat.c
> (svn_ra__locations_from_log): Make a copy of constant apr_array_header_t
> argument before sorting it.
[...]
> @@ -337,10 +338,11 @@ svn_ra__locations_from_log(svn_ra_sessio
> /* Figure out the youngest and oldest revs (amongst the set of
> requested revisions + the peg revision) so we can avoid
> unnecessary log parsing. */
> - svn_sort__array(location_revisions, compare_revisions);
> - oldest_requested = APR_ARRAY_IDX(location_revisions, 0, svn_revnum_t);
> - youngest_requested = APR_ARRAY_IDX(location_revisions,
> - location_revisions->nelts - 1,
> + sorted_location_revisions = apr_array_copy(pool, location_revisions);
> + svn_sort__array(sorted_location_revisions, compare_revisions);
> + oldest_requested = APR_ARRAY_IDX(sorted_location_revisions, 0, svn_revnum_t);
> + youngest_requested = APR_ARRAY_IDX(sorted_location_revisions,
> + sorted_location_revisions->nelts - 1,
> svn_revnum_t);
> youngest = peg_revision;
> youngest = (oldest_requested > youngest) ? oldest_requested : youngest;
> @@ -352,7 +354,7 @@ svn_ra__locations_from_log(svn_ra_sessio
> /* Populate most of our log receiver baton structure. */
> lrb.kind = kind;
> lrb.last_path = fs_path;
> - lrb.location_revisions = apr_array_copy(pool, location_revisions);
> + lrb.location_revisions = apr_array_copy(pool, sorted_location_revisions);
So now you've copied the array twice?
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com