Re: r27729 breaks API of svn_ra_get_log2

On Tue, May 27, 2008 at 7:34 AM, C. Michael Pilato <cmpilato_at_collab.net> wrote:
> Martin von Gagern wrote:
>>
>> Hi!
>>
>> I haven't seen this in real life yet, but I believe r27737 introduced an
>> inconsistency with the API spec. Accoding to svn_ra.h, the paths parameter
>> of svn_ra_get_log2 may be NULL, but as of that patch, the implementation in
>> ra_loader.c dereferences paths in paths->nelts without checking for NULL
>> first.

Actually, looks like it was r27729

>> Do you agree this is a bug?
>
> Yes. (An oversight on my part.)
>
>> Would you like to see this in the issue tracker?
>
> Not if the following patch fixes it.
>
> Index: subversion/libsvn_ra/ra_loader.c
> ===================================================================
> --- subversion/libsvn_ra/ra_loader.c (revision 31465)
> +++ subversion/libsvn_ra/ra_loader.c (working copy)
> @@ -1023,7 +1023,7 @@
> apr_pool_t *pool)
> {
> int i;
> - for (i = 0; i < paths->nelts; i++)
> + for (i = 0; paths && (i < paths->nelts); i++)
> {
> const char *path = APR_ARRAY_IDX(paths, i, const char *);
> assert(*path != '/');

I think putting the whole loop in an if would be more clear.

svn_ra_get_log needs the same change; maybe more?

--dave

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org