bugGNU patch - Bugs: bug #39409, Applying context diff (with diff...

 
 

bug #39409: Applying context diff (with diff -C0) fails.

Submitter:  None
Submitted:  Thu 04 Jul 2013 02:28:19 AM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  None Status:  None
Privacy:  Public Assigned to:  None
Open/Closed:  Open
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 23 Dec 2019 11:30:59 PM UTC, comment #6: 

Hmm, interesting.  Thanks for tracking this down.

In the early days, changes were tracked in rcs (see the rcs-history branch in git).  Individual changes weren't isolated very well.  The commit message pertaining to this change doesn't seem very helpful:

> (another_hunk): Keep context as LINENUM, not int. If the replacement is missing, calculate
> its context correctly. Don't  assume input ends in newline. Keep track of patch prefix
> context separately from suffix context; this fixes several bugs.  Don't assume blank lines
> got chopped if the replacement is missing. Report poorly-formed hunks instead of aborting.
> Do not use strcpy on overlapping strings; it's not portable. Work even if lines are
> incomplete. Fix bugs associated with context-less context hunks, particularly when patching
> in reverse.


The coding style you're proposing looks fine and seems consistent with the rest of the code, so I'm fine with that.

As to what that check in patch-2.1 was for, the pattern and replacement sections of a hunk usually indicate the offset (line number) and the length of the section.  When the length is omitted, it is assumed to be 1.  I think the check is meant to detect when the length was assumed to be 1 and it then turns out that that guess was wrong.  (Some more thinking needed.)

Andreas Gruenbacher <agruen>
Group administrator
Mon 23 Dec 2019 03:27:28 AM UTC, comment #5: 

Since I wanted the test cases to pass when building patch-2.7.6, I downloaded and built patch-2.1, which doesn't have this bug, and triaged the diffs against v2.7.6 for the function another_hunk(), which incorrectly reports the mangled hunk both in the reporter's test case and in the context-format test included in the package.

I found that the problem was fixed by adapting lines 702-707 from pch.c v2.1, which don't have an equivalent, apart from the comment, in v2.7.6:


            /* redundant 'new' context lines were omitted - set */
            /* up to fill them in from the old file context */
            if (!p_context && p_repl_lines == 1) {
                p_repl_lines = 0;
                p_max--;
            }


Although it would be stretching the truth to say that I really came to understand how this 600-line function (now 25% bigger) works, the equivalent of line 703 appears to be


            if (!p_prefix_context && !p_suffix_context && p_repl_lines == 1) {


Personally, I'd like to see arithmetic tests coded explicitly


            if (p_prefix_context != 0 && p_suffix_context != 0 && p_repl_lines == 1) {


but I created a patch in line with the style used in the existing code.
 
[Personally, I'd like the code to have a specification, to be properly commented when, as in this package, C is being used like an assembly language, to match the data being processed, and so on, but hey, if it was good enough for Perl ...]

With pch.c updated by this patch (and the context-format test removed from the XFAIL list), all the PASS test cases pass.

(file #48104)

/df <dirkf>
Thu 07 May 2015 12:00:35 PM UTC, comment #4: 

It seems that you missunderstand my comment.

The gpatch sources have been modified too much and I also
modified Larry's patch sources. It seems to be hard to
compare my POSIX patch and gpatch to find the cause.

I recommend you to go back to patch-2.0-PL12u10
to find out when/how gpatch introduced the bug. You could
use the revision control to get older versions than 2.5...




Jörg Schilling <schily>
Thu 07 May 2015 11:14:04 AM UTC, comment #3: 

So you're telling me, in the GNU patch bug tracker, that other implementations of the patch algorithm don't have this bug which GNU patch has since its earliest archived version (2.5).  Fair enough, send a fix.

Andreas Gruenbacher <agruen>
Group administrator
Thu 07 May 2015 10:21:58 AM UTC, comment #2: 

You are mistaken, the problem does not exist in the original
patch sources and it does not exist in the POSIX patch that
I created from the original sources, see:

https://sourceforge.net/projects/schilytools/files/

I realized the gpatch problem two days ago when running a
regression test against my POSIX patch and using gpatch as
a reference for comparing the patched results.

Jörg Schilling <schily>
Thu 04 Jul 2013 10:28:10 AM UTC, comment #1: 

Interesting -- this bug seems to exist in all versions of patch.

Andreas Gruenbacher <agruen>
Group administrator
Thu 04 Jul 2013 02:28:19 AM UTC, original submission:  

This occur in 2.6.1 (Ubuntu) and 2.7.1 (Cygwin).

$ echo -ne "a\nb\n" > 1.txt
$ echo -ne "a\n" > 2.txt
$ diff -C0 1.txt 2.txt > 12.patch
$ patch 1.txt 12.patch
patch: ** replacement text or line numbers mangled in hunk at line 4
$ patch -R 2.txt 12.patch
patch: ** replacement text or line numbers mangled in hunk at line 4

This doesn't occur if the order of the input is exchanged.

$ echo -ne "a\nb\n" > 1.txt
$ echo -ne "a\n" > 2.txt
$ diff -C0 2.txt 1.txt > 21.patch
$ patch 2.txt 21.patch
patching file 2.txt
$ patch -R 1.txt 21.patch
patching file 1.txt

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #48104:  bug39409.diff added by dirkf (1KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by dirkf (Updated the item)
  • -email is unavailable- added by schily (Posted a comment)
  • -email is unavailable- added by agruen (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-12-23 dirkf Attached File- Added bug39409.diff, #48104
    2019-07-16 agruen Assigned toagruen None
    2013-07-04 agruen Assigned toNone agruen

    Back to the top

    Powered by Savane 3.13-cf05.
    Corresponding source code