Daniel Shahaf wrote: > Julian Foad wrote on Wed, Oct 31, 2012 at 17:36:09 +0000: >> Prabhu Gnana Sundar <prabh...@collab.net> wrote: >> > + revstr = ""; >> > + svn_handle_error2(notify->err, stderr, FALSE /* non-fatal */, >> > + apr_psprintf(scratch_pool, "svnadmin: %s", revstr)); >> > + return; >> >> In what cases will the revision number be invalid? This prints a >> half-empty message in those cases; what did you intend? > > The code will print > svnadmin: E160004: Corrupt filesystem > or > svnadmin: Error verifying revision 42: E160004: Corrupt filesystem
> respectively as the revision number is SVN_INVALID_REVNUM or 42. So > there is no "half-empty" message here. Oh, I see: that psprintf is the 'prefix' argument of handle_error2. I misunderstood. > But the code moves the E160004 away from its current location > immediately after the "svnadmin:" prefix. I am not sure I like that; > is there a good reason not to have the message be of the form > svnadmin: E160004: %s > in the interest of parseability? I agree that would be better: the prefix should remain just "svnadmin: " and the error message should be adjusted instead. - Julian