On Fri, Jan 28, 2011 at 9:23 PM, Daniel Shahaf <d...@daniel.shahaf.name> wrote: > hwri...@apache.org wrote on Fri, Jan 28, 2011 at 20:01:35 -0000: >> Author: hwright >> Date: Fri Jan 28 20:01:35 2011 >> New Revision: 1064847 >> >> URL: http://svn.apache.org/viewvc?rev=1064847&view=rev >> Log: >> * subversion/svnserve/serve.c >> (log_cmd): Remove a useless check, and replace it with an assert instead. >> >> Modified: >> subversion/trunk/subversion/svnserve/serve.c >> >> Modified: subversion/trunk/subversion/svnserve/serve.c >> URL: >> http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/serve.c?rev=1064847&r1=1064846&r2=1064847&view=diff >> ============================================================================== >> --- subversion/trunk/subversion/svnserve/serve.c (original) >> +++ subversion/trunk/subversion/svnserve/serve.c Fri Jan 28 20:01:35 2011 >> @@ -2008,18 +2008,17 @@ static svn_error_t *log_cmd(svn_ra_svn_c >> revprops = NULL; >> else if (strcmp(revprop_word, "revprops") == 0) >> { >> + SVN_ERR_ASSERT(revprop_items); >> + >> - if (revprop_items) > > <as far as I can tell> > > The 'protocol' document explicitly allows the tuple to terminate > immediately after the 'revprops' word --- which, is the case where the > assert would fire; therefore, either your new check violates the > documented protocol, or the protocol documentation needs to be fixed. > > </as far as I can tell>
The protocol document is in error: 'revprops' must always be followed by a list, even if it is the empty list, in which case revprop_items on the server is initialized correctly. If 'revprops' is not followed by a list, the server emits a malformed network data error and closes the network connection post haste. I discovered all this by playing around with a python script to hit an svnserve instance with various combinations of arguments to the log command. Given the above, I think we can just remove the assert, as it won't ever trigger. -Hyrum
