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

Re: 1.8 new public API review (mostly) complete.

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Tue, 2 Apr 2013 14:01:31 +0100 (BST)

Ben Reser wrote:
> On Mon, Apr 1, 2013 at 9:01 PM, Ben Reser <ben_at_reser.org> wrote:
>> I changed it to say "a dirent" which fits with our naming scheme for
>> types of paths.

Should be "an absolute dirent", since dirents can also be relative and I understood that these functions require them to be absolute.

>> [...]
>>  Done along with the doc change mentioned above in r1463374.

I did a quick read-through review and found a little error leak in svnserve/serve.c.  (I'm unfamiliar with the code so can only look for low-level issues.)

> @@ -354,15 +360,17 @@
>        const char *case_force_val;

>        /* Canonicalize and add the base onto the authzdb_path (if needed). */
> -      authzdb_path = canonicalize_access_file(authzdb_path, server, pool);
> +      err = canonicalize_access_file(&authzdb_path, server,
> +                                     repos_root, pool);
>
>        /* Same for the groupsdb_path if it is present. */
> -      if (groupsdb_path)
> -        groupsdb_path = canonicalize_access_file(groupsdb_path,
> -                                                 server, pool);
> +      if (groupsdb_path && !err)
> +        canonicalize_access_file(&groupsdb_path, server, repos_root, pool);

Missing "err = " on this line.

> +
> +      if (!err)
> +        err = svn_repos_authz_read2(&server->authzdb, authzdb_path,
> +                                    groupsdb_path, TRUE, pool);
>
> -      err = svn_repos_authz_read2(&server->authzdb, authzdb_path,
> -                                  groupsdb_path, TRUE, repos_root, pool);
>        if (err)
>          {
>            log_server_error(err, server, conn, pool);

- Julian
Received on 2013-04-02 15:02:14 CEST

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