On Wed, Feb 21, 2001 at 05:55:01AM -0600, Karl Fogel wrote:
> joe@tigris.org writes:
> > Log:
> > Fix non-portable use of 'va_list' argument.
> >
> > * svn_error.c (make_error_internal): Don't set the 'message' field
> > of the error object here. (svn_error_create, svn_error_createf): Set it
> > here instead.
>
> How was this non-portable? This method of passing va_list is also
> used elsewhere in the code. If it's causing a problem on some
> platforms, the above change shouldn't have made that problem go
> away...
It's not the use of va_list per se which is non-portable, but this code
makes assumptions about how va_list is implemented: does the error
message I posted from the compiler make it clear?
I'm guessing but I expect in ANSI C you're not allowed to do anything
with a va_list argument apart from call va_arg on it. Maybe this fix
could have been done similarly like
make_error_internal(...) {
char *dummy;
dummy = va_arg(ap, char *);
if (dummy != NULL) {
/* vsprintf stuff */
} else {
/* ignore extra args */
}
}
I'm not sure if it's okay to call va_arg on an va_list argument and then
pass it on to another function, I might guess not. It seems cleaner to
separate the code out rather than trying to hack through the varargs
list to see if it had a NULL as the first argument.
Regards,
joe
> ?,
> -K
>
> > Revision Changes Path
> > 1.53 +16 -16 subversion/subversion/libsvn_subr/svn_error.c
> >
> > Index: svn_error.c
> > ===================================================================
> > RCS file: /cvs/subversion/subversion/libsvn_subr/svn_error.c,v
> > retrieving revision 1.52
> > retrieving revision 1.53
> > diff -u -r1.52 -r1.53
> > --- svn_error.c 2001/02/21 00:21:00 1.52
> > +++ svn_error.c 2001/02/21 00:40:45 1.53
> > @@ -36,13 +36,10 @@
> > make_error_internal (apr_status_t apr_err,
> > int src_err,
> > svn_error_t *child,
> > - apr_pool_t *pool,
> > - const char *message,
> > - va_list ap)
> > + apr_pool_t *pool)
> > {
> > svn_error_t *new_error;
> > apr_pool_t *newpool;
> > - char *permanent_msg;
> >
> > /* Make a new subpool of the active error pool, or else use child's pool. */
> > if (pool)
> > @@ -64,19 +61,9 @@
> > /* Create the new error structure */
> > new_error = (svn_error_t *) apr_pcalloc (newpool, sizeof (*new_error));
> >
> > - /* Copy the message to permanent storage. */
> > - if (ap)
> > - permanent_msg = apr_pvsprintf (newpool, message, ap);
> > - else
> > - {
> > - permanent_msg = apr_palloc (newpool, (strlen (message) + 1));
> > - strcpy (permanent_msg, message);
> > - }
> > -
> > /* Fill 'er up. */
> > new_error->apr_err = apr_err;
> > new_error->src_err = src_err;
> > - new_error->message = permanent_msg;
> > new_error->child = child;
> > new_error->pool = newpool;
> >
> > @@ -259,7 +246,17 @@
> > apr_pool_t *pool,
> > const char *message)
> > {
> > - return make_error_internal (apr_err, src_err, child, pool, message, NULL);
> > + svn_error_t *err;
> > + char *permanent_msg;
> > +
> > + err = make_error_internal (apr_err, src_err, child, pool);
> > +
> > + permanent_msg = apr_palloc (err->pool, (strlen (message) + 1));
> > + strcpy (permanent_msg, message);
> > +
> > + err->message = permanent_msg;
> > +
> > + return err;
> > }
> >
> >
> > @@ -274,8 +271,11 @@
> > svn_error_t *err;
> >
> > va_list ap;
> > +
> > + err = make_error_internal (apr_err, src_err, child, pool);
> > +
> > va_start (ap, fmt);
> > - err = make_error_internal (apr_err, src_err, child, pool, fmt, ap);
> > + err->message = apr_pvsprintf (err->pool, fmt, ap);
> > va_end (ap);
> >
> > return err;
> >
> >
> >
Received on Sat Oct 21 14:36:22 2006