Re: [Xen-devel] [RFC] xen: Add .astylerc for automated style-formatting

[Viktor Mitin <viktor.mitin.19@xxxxxxxxx>]

On Fri, Aug 2, 2019 at 12:23 PM Juergen Gross <jgross@xxxxxxxx> 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.

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.

Thanks

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel