Questions about code in svn_client_log5()'s helper func resolve_log_targets()

Paul,

Was reviewing your svn_client_log5() changes. There are a couple of places
in your reworked svn_client_log5() code (resolve_log_targets(),
specifically) that read like so or similar:

       if (peg_revision->kind == svn_opt_revision_unspecified)
           (*peg_revision).kind = svn_opt_revision_head;

Is there any reason for that not to be simply:

       if (peg_revision->kind == svn_opt_revision_unspecified)
         peg_revision->kind = svn_opt_revision_head;

?

Also, I would suggest that, instead of initializing the *relative_targets
array with a single slot, we go ahead and use the number of slots we know
we'll need (so we can avoid resizing the array later):

  /* Initialize the output array. At a minimum, we need room for one
     (possibly empty) relpath. Otherwise, we have to hold a relpath
     for every item in TARGETS except the first. */
  *relative_targets = apr_array_make(result_pool,
                                     MAX(1, targets->nelts - 1),
                                     sizeof(const char *));

Finally, do we need to be strdup()ing the stuff we put into the
relative_targets array? Looks to me (via code inspection) like perhaps not.

It's entirely possible that you didn't introduce any of this stuff with your
recent code reorg here -- I'm not claiming otherwise -- but I wanted to make
sure you agreed with me before I started messing around in that relatively
complex bit of code.

-- 
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development