Evgeny Kotkov <evgeny.kotkov_at_visualsvn.com> writes:
> Our last Subversion 1.9.2 release happened about two months ago, and we
> have several important fixes waiting to be released.
>
> I plan to roll 1.8.15 and 1.9.3 next Monday, on December 7th. So please
> wrap up any nomination/voting for things that you'd like included.
Unfortunately, I was unable to roll the 1.9.3 tarball, since the build failed
on my Ubuntu 14.04.1 LTS using gcc 4.8.2 (Ubuntu 4.8.2-19ubuntu1):
[[[
In file included from /usr/include/unistd.h:1148:0,
from subversion/mod_dav_svn/status.c:99:
/usr/include/x86_64-linux-gnu/bits/unistd.h: In function ‘dav_svn__status’:
/usr/include/x86_64-linux-gnu/bits/unistd.h:34:1: error: nested function
‘read’ declared ‘extern’
read (int __fd, void *__buf, size_t __nbytes)
^
/usr/include/x86_64-linux-gnu/bits/unistd.h:34:1: error: static declaration
of ‘read’ follows non-static declaration
In file included from subversion/mod_dav_svn/status.c:99:0:
/usr/include/unistd.h:360:16: note: previous declaration of ‘read’ was here
extern ssize_t read (int __fd, void *__buf, size_t __nbytes) __wur;
^
...
]]]
The build failure bisects to r1709553 (merged to /branches/1.9.x in r1713077):
* r1709553
Fix display of process ID in mod_dav_svn cache statistics.
Justification:
User cannot determine to which process the output applies.
With this change, the #if in /subversion/mod_dav_svn/status.c evaluates to
true, and that results in including <unistd.h> right in the middle of the
dav_svn__status() function. From my reading of §7.1.2/4, this is UB, and
that explains why the code might compile, but can as well fail to do so:
[[[
7.1.2/4 If used, a header shall be included outside of any external
declaration or definition, and it shall first be included before the first
reference to any of the functions or objects it declares, or to any of the
types or macros it defines.
]]]
In case there are no objections, I'll revert r1713077 from /branches/1.9.x
and try to roll 1.9.3 again. We can then fix the problem in trunk and backport
the whole fix again, once it's ready.
As for fixing the problem, I think that we could do something like this:
[[[
Index: subversion/mod_dav_svn/status.c
===================================================================
--- subversion/mod_dav_svn/status.c (revision 1718265)
+++ subversion/mod_dav_svn/status.c (working copy)
@@ -47,6 +47,10 @@
#endif
#include "svn_private_config.h"
+#ifdef HAVE_UNISTD_H
+#include <unistd.h> /* For getpid() */
+#endif
+
#ifndef DEFAULT_TIME_FORMAT
#define DEFAULT_TIME_FORMAT "%Y-%m-%d %H:%M:%S %Z"
#endif
@@ -96,7 +100,6 @@ int dav_svn__status(request_rec *r)
the request. Ideally we would iterate over all processes but that
would need some MPM support, so we settle for simply showing the
process ID. */
-#include <unistd.h>
ap_rprintf(r, "<dt>Server process id: %d</dt>\n", (int)getpid());
#endif
]]]
Regards,
Evgeny Kotkov
Received on 2015-12-07 18:59:09 CET