Re: Messages from hooks

Ben Collins-Sussman <sussman@collab.net> writes:

> Using trunk@HEAD code, I don't get a segfault like you, nor do I get
> that weird bit of corrupted text in the errorlog (!õ #?ö ï). Instead, I
> see:

After a bit of debugging and some valgrind runs I now think there are
two bugs. The first is a trunk@HEAD only problem and it is the one
that caused the segfault and dodgy characters. It's a use of memory
after a pool has been cleared and I can fix it using this patch which
uses a pool with a longer lifetime (it also always uses the fixed
error text for two reasons: we don't want two errors with the same
text and the serr->message pool is about to be destroyed):

Index: subversion/mod_dav_svn/deadprops.c
===================================================================
--- subversion/mod_dav_svn/deadprops.c (revision 8758)
+++ subversion/mod_dav_svn/deadprops.c (working copy)
@@ -158,9 +158,8 @@
                                          propname, value, db->resource->pool);
   if (serr != NULL)
     return dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
- serr->message ?
- serr->message : "could not change a property",
- db->p);
+ "could not change a property",
+ db->resource->pool);
 
   /* a change to the props was made; make sure our cached copy is gone */
   db->props = NULL;

There are two other dav_svn_convert_err in this file, I think they
should also use db->resource->pool but all this DAV stuff is a bit
alien to me. I think I'll commit a change for all three onto the
trunk.

The second bug is present on both the trunk and the 1.0.x branch, but
it's not so serious. It's the bug responsible for the "Invalid or
incomplete multibyte or wide character"

> [Thu Feb 19 11:02:19 2004] [error] [client 127.0.0.1] (84)Invalid or
> incomplete multibyte or wide character: 'pre-revprop-change' hook
> failed with error output:

This is caused by the (84) which is the value of the global errno when
we call dav_svn_convert_err! It's not clear to me what we should do
about this. errno is only valid when a system call fails, otherwise
the value is undefined. In this case the value of errno changes from
2 to 84 when we run the hook, but it does that whether or not the hook
returns an error, i.e. if I change my hook to "exit 0" I still get
errno set to 84. I don't know whether it is better for
dav_svn_convert_err to set errno to zero, possibly losing information,
or to leave it alone, possibly providing erroneous information.

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org