Re: [PATCH v3 2/2] attr: add flag `-r|--revisions` to work with revisions

From: Karthik Nayak <karthik.188@gmail.com>
Date: Dec 17, 2022, 15:27 UTC
Message-ID: <CAOLa=ZTXQ6e_JY+3jN6r52j71qTo5-OJBs-_vLaE_obn+3Fb1Q@mail.gmail.com>
In-Reply-To: <xmqqcz8ikgxs.fsf@gitster.g>

On Sat, Dec 17, 2022 at 1:33 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 19 quoted lines

>
> > diff --git a/attr.c b/attr.c
> > index 42ad6de8c7..99883024ec 100644
> > --- a/attr.c
> > +++ b/attr.c
> > @@ -11,8 +11,13 @@
> >  #include "exec-cmd.h"
> >  #include "attr.h"
> >  #include "dir.h"
> > +#include "git-compat-util.h"
>
> Unneeded.  cf. Documentation/CodingGuidelines
>
>  - The first #include in C files, except in platform specific compat/
>    implementations, must be either "git-compat-util.h", "cache.h" or
>    "builtin.h".  You do not have to include more than one of these.
>
> and this file already begins with including "cache.h".
>

Thanks, I thought this was removed.

Show 29 quoted lines

> By the way,
>
>     $ make
>     $ cd t
>     $ sh t0003-attributes.sh -i -x
>     Initialized empty Git repository in /home/gitster/w/git.git/t/trash directory.t0003-attributes/.git/
>     expecting success of 0003.1 'open-quoted pathname':
>             echo "\"a test=a" >.gitattributes &&
>             attr_check a unspecified
>
>     ++ echo '"a test=a'
>     ++ attr_check a unspecified
>     ++ attr_check_basic a unspecified
>     ++ path=a
>     ++ expect=unspecified
>     ++ git_opts=
>     ++ git check-attr test -- a
>     t0003-attributes.sh: line 9: 1508946 Segmentation fault      git $git_opts check-attr test -- "$path" > actual 2> err
>     error: last command exited with $?=139
>     not ok 1 - open-quoted pathname
>     #
>     #           echo "\"a test=a" >.gitattributes &&
>     #           attr_check a unspecified
>     #
>     1..1
>     $ exit
>
> there seems to be something fishy in this patch.
>

Seems to be because tree_oid is not NULL initialized. I think I only tested the new feature, but the other tests are failing. Should be fixed with

    -+  struct object_id tree_oid;
    ++  struct object_id *tree_oid = NULL;
        int cnt, i, doubledash, filei;

        if (!is_bare_repository())
    @@ builtin/check-attr.c: int cmd_check_attr(int argc, const char
**argv, const char
                }
        }

    -+  if (revision)
    -+          if (repo_get_oid_tree(the_repository, revision, &tree_oid))
    ++  if (revision) {
    ++          tree_oid = xmalloc(sizeof(struct object_id));
    ++
    ++          if (repo_get_oid_tree(the_repository, revision, tree_oid))
     +                  error("%s: not a valid revision", revision);
    ++  }

will include in version 4. Thanks for the support, Junio.

-- 
- Karthik