You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by kf...@collab.net on 2004/08/16 20:02:53 UTC
issue #2006: severe new bug, help requested
I'd like to draw developers' attention to issue #2006, a severe bug in
'svn log -v' over DAV. It seems to have entered the trunk codebase
relatively recently. It is easy to reproduce; see the script in
http://subversion.tigris.org/issues/show_bug.cgi?id=2006
If you have a chance, please at least glance over the issue and see if
anything about it jumps out at you. Ben Collins-Sussman and I are
stumped so far (he just discovered it today).
Thanks,
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: issue #2006: severe new bug, help requested
Posted by Philip Martin <ph...@codematters.co.uk>.
kfogel@collab.net writes:
> We'd have to define things in such a way that
>
> __attribute__(x)
>
> expands to empty, for non-GCC though, right?
Yes.
> Is that taken care of automagically?
Include apr.h.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: issue #2006: severe new bug, help requested
Posted by kf...@collab.net.
Philip Martin <ph...@codematters.co.uk> writes:
> > It's a lot of call sites to change in Subversion, though. Not sure
> > it's worth the churn or the ugliness.
>
> It's a change to the function declaration, not the call sites.
Indeed, I read too hastily.
Surely we only have a few varargs functions in Subversion, so this is
doable. We'd have to define things in such a way that
__attribute__(x)
expands to empty, for non-GCC though, right? Is that taken care of
automagically?
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: issue #2006: severe new bug, help requested
Posted by Philip Martin <ph...@codematters.co.uk>.
kfogel@collab.net writes:
> Neat. (Is that how GCC/libc handles 'printf' specially?)
I believe it's the same code that does the check, but gcc knows to
check printf without an __attribute__ qualifier.
> It's a lot of call sites to change in Subversion, though. Not sure
> it's worth the churn or the ugliness.
It's a change to the function declaration, not the call sites.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: issue #2006: severe new bug, help requested
Posted by kf...@collab.net.
Philip Martin <ph...@codematters.co.uk> writes:
> If we were to use something like:
>
> Index: subversion/mod_dav_svn/dav_svn.h
> ===================================================================
> --- subversion/mod_dav_svn/dav_svn.h (revision 10664)
> +++ subversion/mod_dav_svn/dav_svn.h (working copy)
> @@ -493,7 +493,8 @@
> /* Output XML data to OUTPUT using BB. Use FMT as format string for the.
> output. */
> svn_error_t * dav_svn__send_xml(apr_bucket_brigade *bb, ap_filter_t *output,
> - const char *fmt, ...);
> + const char *fmt, ...)
> + __attribute__((format(printf,3,4)));
>
> then gcc would produce a warning:
>
> /home/pm/sw/subversion/svn/subversion/mod_dav_svn/log.c: In function `log_receiver':
> /home/pm/sw/subversion/svn/subversion/mod_dav_svn/log.c:179: warning: too many arguments for format
Neat. (Is that how GCC/libc handles 'printf' specially?)
It's a lot of call sites to change in Subversion, though. Not sure
it's worth the churn or the ugliness.
What I *really* want to know is, how come log_tests.py didn't fail
when run over dav? (I'm pretty sure I've run davcheck since r10362.)
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: issue #2006: severe new bug, help requested
Posted by Philip Martin <ph...@codematters.co.uk>.
kfogel@collab.net writes:
> Philip Martin <ph...@codematters.co.uk> writes:
>> Index: subversion/mod_dav_svn/log.c
>> ===================================================================
>> --- subversion/mod_dav_svn/log.c (revision 10664)
>> +++ subversion/mod_dav_svn/log.c (working copy)
>> @@ -177,7 +177,7 @@
>>
>> case 'M':
>> SVN_ERR( dav_svn__send_xml(lrb->bb, lrb->output,
>> - "<S:modified-path>%s",
>> + "<S:modified-path>%s"
>> "</S:modified-path>" DEBUG_CR,
>> apr_xml_quote_string(pool, path, 0)) );
>> break;
>
> Holy cow.
>
> Uh, thank you, Philip.
If we were to use something like:
Index: subversion/mod_dav_svn/dav_svn.h
===================================================================
--- subversion/mod_dav_svn/dav_svn.h (revision 10664)
+++ subversion/mod_dav_svn/dav_svn.h (working copy)
@@ -493,7 +493,8 @@
/* Output XML data to OUTPUT using BB. Use FMT as format string for the.
output. */
svn_error_t * dav_svn__send_xml(apr_bucket_brigade *bb, ap_filter_t *output,
- const char *fmt, ...);
+ const char *fmt, ...)
+ __attribute__((format(printf,3,4)));
then gcc would produce a warning:
/home/pm/sw/subversion/svn/subversion/mod_dav_svn/log.c: In function `log_receiver':
/home/pm/sw/subversion/svn/subversion/mod_dav_svn/log.c:179: warning: too many arguments for format
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: issue #2006: severe new bug, help requested
Posted by kf...@collab.net.
Philip Martin <ph...@codematters.co.uk> writes:
> Index: subversion/mod_dav_svn/log.c
> ===================================================================
> --- subversion/mod_dav_svn/log.c (revision 10664)
> +++ subversion/mod_dav_svn/log.c (working copy)
> @@ -177,7 +177,7 @@
>
> case 'M':
> SVN_ERR( dav_svn__send_xml(lrb->bb, lrb->output,
> - "<S:modified-path>%s",
> + "<S:modified-path>%s"
> "</S:modified-path>" DEBUG_CR,
> apr_xml_quote_string(pool, path, 0)) );
> break;
Holy cow.
Uh, thank you, Philip.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: issue #2006: severe new bug, help requested
Posted by Philip Martin <ph...@codematters.co.uk>.
kfogel@collab.net writes:
> I'd like to draw developers' attention to issue #2006, a severe bug in
> 'svn log -v' over DAV. It seems to have entered the trunk codebase
> relatively recently. It is easy to reproduce; see the script in
>
> http://subversion.tigris.org/issues/show_bug.cgi?id=2006
>
> If you have a chance, please at least glance over the issue and see if
> anything about it jumps out at you. Ben Collins-Sussman and I are
> stumped so far (he just discovered it today).
Index: subversion/mod_dav_svn/log.c
===================================================================
--- subversion/mod_dav_svn/log.c (revision 10664)
+++ subversion/mod_dav_svn/log.c (working copy)
@@ -177,7 +177,7 @@
case 'M':
SVN_ERR( dav_svn__send_xml(lrb->bb, lrb->output,
- "<S:modified-path>%s",
+ "<S:modified-path>%s"
"</S:modified-path>" DEBUG_CR,
apr_xml_quote_string(pool, path, 0)) );
break;
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org