[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Can err->message be NULL?

From: <kfogel_at_collab.net>
Date: 2005-07-06 20:41:58 CEST

Greg Hudson <ghudson@MIT.EDU> writes:
> On Tue, 2005-07-05 at 14:25 -0500, kfogel@collab.net wrote:
> > Our documentation for 'svn_error_t' does not specify whether the
> > message field can ever be NULL. Our code is inconsistent about
> > checking for this before using err->message.
> >
> > Do we want to have a policy? If so, which?
>
> >From the svn_error_create docstring:
>
> * Notes: Errors are always allocated in a subpool of the global pool,
> * since an error's lifetime is generally not related to the
> * lifetime of any convenient pool. Errors must be freed
> * with @c svn_error_clear(). The specific message should be NULL
> * if there is nothing to add to the general message associated
> * with the error code.
>
> So yes, it can be NULL. Where are we failing to check?

A few places. Do a tags-search for "err->message" (and throw in
"err2->message" if you're feeling squirrelly) to see them all. A few
examples:

In mod_authz_svn/mod_authz_svn.c:

            ap_log_rerror(APLOG_MARK, APLOG_ERR,
                          /* If it is an error code that APR can make sense
                             of, then show it, otherwise, pass zero to avoid
                             putting "APR does not understand this error code"
                             in the error log. */
                          ((svn_err->apr_err >= APR_OS_START_USERERR &&
                            svn_err->apr_err < APR_OS_START_CANONERR) ?
                           0 : svn_err->apr_err),
                          r, "Failed to load the AuthzSVNAccessFile: %s",
                          svn_err->message);

...and various similar examples in that file.

In mod_dav_svn/util.c:

    derr = dav_svn__new_error_tag(pool, status, serr->apr_err,
                                  apr_pstrdup(pool, serr->message),
                                  SVN_DAV_ERROR_NAMESPACE,
                                  SVN_DAV_ERROR_TAG);

...and in various other places in mod_dav_svn, though note that calls
to dav_svn_convert_error() can accept NULL as the third parameter, so
they're okay to use err->message without checking. The above snippet
from dav_svn_convert_err() is not okay, however, because it's about
'serr->message' not just 'message'.

In libsvn_subr/error.c (ironically enough):

   void
   svn_handle_warning2 (FILE *stream, svn_error_t *err, const char *prefix)
   {
     svn_error_clear (svn_cmdline_fprintf (stream, err->pool,
                                           _("%swarning: %s\n"),
                                           prefix, err->message));
     fflush (stream);
   }

I suppose I could just jump in and fix these, but I don't want to
leave code review right now, so flog me... :-)

-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jul 6 21:30:01 2005

This is an archived mail posted to the Subversion Dev mailing list.