On Wed, Jun 29, 2011 at 3:26 AM, Julian Foad <julian.f...@wandisco.com> wrote: > On Wed, 2011-06-29 at 02:16 +0300, Daniel Shahaf wrote: >> hwri...@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.
Correct, that was my logic (and you explained it better than I would have). -Hyrum
