On Wed, 2011-06-29 at 02:16 +0300, Daniel Shahaf wrote:
> hwright_at_apache.org wrote on Tue, Jun 28, 2011 at 21:47:06 -0000:
> > Author: hwright
> > Date: Tue Jun 28 21:47:06 2011
> > New Revision: 1140861
> >
> > URL: http://svn.apache.org/viewvc?rev=1140861&view=rev
> > Log:
> > * subversion/svnadmin/main.c
> > (parse_args): Fix a theoretical bug by ensure we only attempt to parse args
> > if we think we have them.
> >
> > Modified:
> > subversion/trunk/subversion/svnadmin/main.c
> >
> > Modified: subversion/trunk/subversion/svnadmin/main.c
> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnadmin/main.c?rev=1140861&r1=1140860&r2=1140861&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/svnadmin/main.c (original)
> > +++ subversion/trunk/subversion/svnadmin/main.c Tue Jun 28 21:47:06 2011
> > @@ -559,9 +559,11 @@ parse_args(apr_array_header_t **args,
> > if (args)
> > {
> > *args = apr_array_make(pool, num_args, sizeof(const char *));
> > - while (os->ind < os->argc)
> > - APR_ARRAY_PUSH(*args, const char *) =
> > - apr_pstrdup(pool, os->argv[os->ind++]);
> > +
> > + if (num_args)
> > + while (os->ind < os->argc)
> > + APR_ARRAY_PUSH(*args, const char *) =
> > + apr_pstrdup(pool, os->argv[os->ind++]);
> > }
> >
>
> I don't understand this change. The loop would execute zero times when
> os->ind == os->argc, so what are you guarding against?
>
> I think you forgot to either:
>
> * check 'os != NULL'
>
> * move the apr_array_make() inside the check that NUM_ARGS > 0
Actually, Hyrum's code looks correct to me. It guards against 'os'
because 'num_args' is zero if 'os' is null:
int num_args = os ? (os->argc - os->ind) : 0;
And the array needs to be allocated if the caller requested it, even if
it is going to have no elements.
- Julian
Received on 2011-06-29 10:32:35 CEST