Bert Huijben wrote on Wed, May 22, 2013 at 10:50:17 +0200:
>
>
> > -----Original Message-----
> > From: Philip Martin [mailto:[email protected]]
> > Sent: woensdag 22 mei 2013 10:46
> > To: [email protected]
> > Subject: Re: svn commit: r1485007 - /subversion/branches/invoke-diff-cmd-
> > feature/subversion/libsvn_subr/io.c
> >
> > [email protected] writes:
> >
> > > Author: gbg
> > > Date: Tue May 21 22:57:23 2013
> > > New Revision: 1485007
> > >
> > > URL: http://svn.apache.org/r1485007
> > > Log:
> > > On the invoke-diff-cmd branch: Repair erroneous initialization.
> > >
> > > * subversion/svn/io.c
> > >
> > > (svn_io_run_external_diff): Change pcalloc to palloc call.
> > >
> > > Modified:
> > > subversion/branches/invoke-diff-cmd-
> > feature/subversion/libsvn_subr/io.c
> > >
> > > Modified: subversion/branches/invoke-diff-cmd-
> > feature/subversion/libsvn_subr/io.c
> > > URL: http://svn.apache.org/viewvc/subversion/branches/invoke-diff-cmd-
> > feature/subversion/libsvn_subr/io.c?rev=1485007&r1=1485006&r2=1485007
> > &view=diff
> > >
> > ==========================================================
> > ====================
> > > --- subversion/branches/invoke-diff-cmd-
> > feature/subversion/libsvn_subr/io.c (original)
> > > +++ subversion/branches/invoke-diff-cmd-
> > feature/subversion/libsvn_subr/io.c Tue May 21 22:57:23 2013
> > > @@ -3036,7 +3036,7 @@ svn_io_run_external_diff(const char *dir
> > > for (i = 0, size = 0; cmd[i]; i++)
> > > size += strlen(cmd[i]) + 1;
> > >
> > > - failed_command = apr_pcalloc(pool, size * sizeof(char *));
> > > + failed_command = apr_palloc(pool, size * sizeof(char *));
> > >
> > > for (i = 0; cmd[i]; i++)
> > > {
> > >
> >
> > That's not right. You have calculated 'size' by summing strlen to give
> > you the total text length. Multiplying the text length by the sizeof a
> > pointer is wrong. You could multiply by sizeof(char) but that is 1. You
> > need to add +1 for the terminating null byte.
> >
> > However this is an error path so I'd go for simplicity rather than
> > efficiency:
> >
> > const char *failed_command = "";
> > for (i = 0; cmd[i]; ++i)
> > {
> > failed_command = apr_pstrcat(pool, failed_command, cmd[i]);
> > failed_command = apr_pstrcat(pool, failed_command, " ");
>
> Note that apr_pstrcat needs a final NULL argument :)
>
> So you could use apr_pstrcat(pool, failed_command, " ", cmd[i], NULL);
You need to explicitly cast the last NULL:
apr_pstrcat(pool, failed_command, " ", cmd[i], (void *)NULL);
(because it's a variadic function so there is no implicit conversion to
a pointer, when NULL is #define'd to be a 0 narrower than the pointer)
