Re: [bug-patch] patch-2.5.9 - patch.c:340: main: Assertion `hunk' failed.

[Alexander Lamaison]

2009/6/6 Andreas Gruenbacher <address@hidden>:
> On Monday, 1 June 2009 13:42:08 Alexander Lamaison wrote:
>> While transforming the patch code into a reusable library, I came
>> across a bug that is also present in the mainstream patch code.
>>
>> When given an input appears to be a standard diff but has inconsistent
>> indentation (attached as test_data), intuit_diff_type() returns
>> NORMAL_DIFF as the type and sets the second line's indentation as the
>> indent for the whole file.  there_is_another_patch()  continues to
>> process the file but fails to find any hunks, presumably, because it
>> gets confused by the indentation.  Eventually this causes the
>> following assertion:
>>
>> $ ./patch --dry-run < test_input
>> can't find file to patch at input line 1
>> Perhaps you should have used the -p or --strip option?
>> File to patch: patch
>> patching file patch
>> patch: patch.c:340: main: Assertion `hunk' failed.
>> Aborted
>
> very interesting. The assertion that triggers here has been removed as part of
> making patch act more reasonable for empty patches [1]. The disagreement
> between intuit_diff_type() and another_hunk() still hasn't been fixed though,
> and so in the latest version of the code, instead of aborting, we end up in
> an endless loop. This needs fixing.

I couldn't repro this problem (either an assertion or infinite loop)
with the alpha snapshots:
    $ ./patch --dry-run < test_input2.txt
    ./patch: **** Only garbage was found in the patch input.
It seems this is being caught earlier in the program now.

> I don't see how you ended up checking for (p_indent > 0 && p_indent !=
> indent).

My theory was that if p_indent had already been set for an earlier
line (which I took to be p_indent > 0) then it shouldn't be
contradicted.  However of course, if p_indent is 0 in this block that
just means it was set to 0 on an earlier line and therefore still
shouldn't be contradicted.  Your patch does this correctly.  Why was
p_indent ever set in this block?

> Shouldn't p_indent and indent always match to recognize a normal diff? How
> about something like this?
>
> diff --git a/src/pch.c b/src/pch.c
> index 0a136ae..2bf8066 100644
> --- a/src/pch.c
> +++ b/src/pch.c
> @@ -570,10 +570,10 @@ intuit_diff_type (bool need_header)
>        }
>        if ((diff_type == NO_DIFF || diff_type == NORMAL_DIFF) &&
>          last_line_was_command &&
> +         p_indent == indent &&
>          (strnEQ(s, "< ", 2) || strnEQ(s, "> ", 2)) ) {
>            p_start = previous_line;
>            p_sline = p_input_line - 1;
> -           p_indent = indent;
>            p_strip_trailing_cr = strip_trailing_cr;
>            retval = NORMAL_DIFF;
>            goto scan_exit;
>

If intuit_diff_type() reaches the end of all these checks without
retval being set, what happens?  Are enums initialised to a particular
value?

> GNU patch development has recently picked up speed again. The project is
> hosted at http://savannah.gnu.org/projects/patch; working against either the
> git tree or a recent snapshot would make things a little easier for me.

I can seem to build the git version of the code:

    $ autoconf
    configure.ac:52: error: possibly undefined macro: AM_STDBOOL_H
          If this token and others are legitimate, please use m4_pattern_allow.
          See the Autoconf documentation.
    configure.ac:83: error: possibly undefined macro: AC_FUNC_SETMODE_DOS
    configure.ac:87: error: possibly undefined macro: gl_AC_DOS

Maybe I have the wrong version of autoconf.  Which version is required?

    $ autoconf --version
    autoconf (GNU Autoconf) 2.63
    Copyright (C) 2008 Free Software Foundation, Inc.
    License GPLv2+: GNU GPL version 2 or later
    <http://gnu.org/licenses/old-licenses/gpl-2.0.html>
    This is free software: you are free to change and redistribute it.
    There is NO WARRANTY, to the extent permitted by law.

    Written by David J. MacKenzie and Akim Demaille.
    ---
    Autoconf 2.50 chosen by Debian wrapper script.
    For information and tuning advice see autoconf(1).

    $ uname -a
    Linux defiant 2.6.28-12-generic #43-Ubuntu SMP Fri May 1 19:27:06
UTC 2009 i686 GNU/Linux

Alex
--
http://www.doc.ic.ac.uk/~awl03