On 02.08.2019 13:44, Viktor Mitin wrote:
> On Fri, Aug 2, 2019 at 12:23 PM Juergen Gross <jgr...@suse.com> wrote:
>
>>>> A snipplet from commit 57f8b13c724023c78fa15a80452d1de3e51a1418:
>>>>
>>>> @@ -4096,14 +4097,12 @@ retry_transaction:
>>>> goto out;
>>>>
>>>> if (target == NULL) {
>>>> - libxl__xs_printf(gc, t, target_path, "%"PRIu32,
>>>> - (uint32_t) info.current_memkb);
>>>> - *target_memkb = (uint32_t) info.current_memkb;
>>>> + libxl__xs_printf(gc, t, target_path, "%"PRIu64,
>>>> info.current_memkb);
>>>> + *target_memkb = info.current_memkb;
>>>> }
>>>> if (staticmax == NULL) {
>>>> - libxl__xs_printf(gc, t, max_path, "%"PRIu32,
>>>> - (uint32_t) info.max_memkb);
>>>> - *max_memkb = (uint32_t) info.max_memkb;
>>>> + libxl__xs_printf(gc, t, max_path, "%"PRIu64, info.max_memkb);
>>>> + *max_memkb = info.max_memkb;
>>>> }
>>>>
>>>> rc = 0;
>>>>
>>>
>>> I've build gnu diffutils latest release 3.7 and checked the code and
>>> the issue. It seems this feature (-p) doesn't work properly and has
>>> many more bugs than just the issue with labels. See the example below,
>>> the change has been made in the function a1(), however, in the diff
>>> another function is shown a().
>>
>> This case is completely fine, as the context of the diff is starting
>> before the definition of a() (and just after a1()). This is important in
>> case you only add a new function for example.
>>
>
> Juergen, the diff returns wrong name of the function silently. I don't
> think it is 'completely fine', I would rather call it a bug, or
> non-documented limitation, but definitely not a feature. As I wrote
> previously, I played with -p a little and observed more similar issues
> with it. The reason is that the next diff code (see below) tries to
> match function names using simple regexp for a line, assuming that
> line with function name cannot start with a 'non-alpha' characters. So
> even adding one more space before a line with function name breaks it.
And there's not supposed to be any blank ahead of a function's
heading.
> diffutils-3.7/src/diff.c:462
> case 'p':
> show_c_function = true;
> add_regexp (&function_regexp_list, "^[[:alpha:]$_]");
>
> It is probably could be improved by adding 'no : symbol at the end of
> the line' logic to this regexp. It will resolve the issue with labels,
> but will add one more bug (or limitation) about using ":" in such
> lines.
>
> The issue is more general here. The diff tool should not parse any
> programming languages, it should just compare the strings.
"The strings" being which ones? The limitations (if you want to call
them such) of "diff -p" are, I think, well understood. And there's
no "parsing" here at all - the regexp simply checks the first
character. The time when you wanted to make it skip labels would be
where it would become more like "parsing".
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
