[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Wrong assumption about unidiff syntax in libsvn_diff?

From: Stefan Sperling <stsp_at_elego.de>
Date: Tue, 20 Nov 2012 16:09:08 +0100

On Tue, Nov 20, 2012 at 03:40:20PM +0100, Branko Čibej wrote:
> On 20.11.2012 15:05, Stefan Sperling wrote:
> > I just checked the UNIX patch code shipped with OpenBSD and it seems
> > you're right that it only looks for the backslash and ignores the comment.
> >
> > However, it seems in practice patches usually contain this string in
> > non-localised form. At least nobody has yet complained about svn patch
> > misparsing such patches.
>
> I'm distinctly remember a report on this very list, with a unidiff with
> a localized message attached. However, I can't find it in the archives.
> In any case it would be nice if our diff parser only looked at the \,
> not the whole message. I'm less worried about our not localizing that
> particular string.

Currently, lines such as:
\\ this is a comment
anywhere in the patch are silently ignored.

If we follow your suggestion we must treat any such lines as hunk
terminators and assume the hunk lacks a trailing newline.
I suppose such a change is correct but it's a behaviour change so
I wouldn't want to backport it to 1.7.x.

Below is a patch. Diff parser tests and patch tests are still passing.

[[[
* subversion/libsvn_diff/parse-diff.c
  (parse_next_hunk): Treat any line that starts with a backslash as a
   hunk terminator, indicating that the hunk does not end with EOL.
   Comments following the backslash might be localised or missing
   in which case parsing the patch would fail.

Suggested by: brane
]]]

Index: subversion/libsvn_diff/parse-diff.c
===================================================================
--- subversion/libsvn_diff/parse-diff.c (revision 1411078)
+++ subversion/libsvn_diff/parse-diff.c (working copy)
@@ -555,15 +555,11 @@ parse_next_hunk(svn_diff_hunk_t **hunk,
       pos = 0;
       SVN_ERR(svn_io_file_seek(apr_file, APR_CUR, &pos, iterpool));
 
- /* Lines starting with a backslash are comments, such as
+ /* Lines starting with a backslash indicate a missing EOL:
        * "\ No newline at end of file". */
       if (line->data[0] == '\\')
         {
- if (in_hunk &&
- ((!*is_property &&
- strcmp(line->data, "\\ No newline at end of file") == 0) ||
- (*is_property &&
- strcmp(line->data, "\\ No newline at end of property") == 0)))
+ if (in_hunk)
             {
               char eolbuf[2];
               apr_size_t len;
Received on 2012-11-20 16:09:59 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.