Re: [PATCH 04/27] test-read-cache: print cache entries with --table

[Elijah Newren <newren@gmail.com>]

On Mon, Jan 25, 2021 at 9:42 AM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Derrick Stolee <dstolee@microsoft.com>
>
> This table is helpful for discovering data in the index to ensure it is
> being written correctly, especially as we build and test the
> sparse-index.
>
> To make the option parsing slightly more robust, wrap the string
> comparisons in a loop adapted from test-dir-iterator.c.
>
> Care must be taken with the final check for the 'cnt' variable. We
> continue the expectation that the numerical value is the final argument.
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  t/helper/test-read-cache.c | 49 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 44 insertions(+), 5 deletions(-)
>
> diff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c
> index 244977a29bd..cd7d106a675 100644
> --- a/t/helper/test-read-cache.c
> +++ b/t/helper/test-read-cache.c
> @@ -2,18 +2,55 @@
>  #include "cache.h"
>  #include "config.h"
>
> +static void print_cache_entry(struct cache_entry *ce)
> +{
> +       /* stat info */
> +       printf("%08x %08x %08x %08x %08x %08x ",
> +              ce->ce_stat_data.sd_ctime.sec,
> +              ce->ce_stat_data.sd_ctime.nsec,
> +              ce->ce_stat_data.sd_mtime.sec,
> +              ce->ce_stat_data.sd_mtime.nsec,
> +              ce->ce_stat_data.sd_dev,
> +              ce->ce_stat_data.sd_ino);

Printing sec & nsec in hexidecimal?  Why?

Also, if they'll be displayed in hex, do you want to format them as
0x%08x, similar to what you do with binary below?

> +
> +       /* mode in binary */

This comment feels misleading; I think this is the "S_IFMT portion of
mode in binary" not "mode in binary".

> +       printf("0b%d%d%d%d ",
> +               (ce->ce_mode >> 15) & 1,
> +               (ce->ce_mode >> 14) & 1,
> +               (ce->ce_mode >> 13) & 1,
> +               (ce->ce_mode >> 12) & 1);

Why binary?  Also, since you defined a special magic constant of
01000755 which utilizes bit 18; how come you aren't including any bits
higher than 15?

> +       /* output permissions? */
> +       printf("%04o ", ce->ce_mode & 01777);

01777 instead of 07777 just because we don't have anything using the
setuid or setgid bits?  But if it's based on non-use, then we don't
use the sticky bit (01000) either, so this could be just 0777.

Also, if you're using 0b for binary to distinguish and you're clearly
using multiple bases in this code, perhaps use a print format of
0o%04o (or 0o%03o if you only use a mask of 0777).

> +       printf("%s ", oid_to_hex(&ce->oid));
> +
> +       printf("%s\n", ce->name);
> +}
> +
> +static void print_cache(struct index_state *cache)
> +{
> +       int i;
> +       for (i = 0; i < the_index.cache_nr; i++)
> +               print_cache_entry(the_index.cache[i]);
> +}
> +
>  int cmd__read_cache(int argc, const char **argv)
>  {
>         int i, cnt = 1;
>         const char *name = NULL;
> +       int table = 0;
>
> -       if (argc > 1 && skip_prefix(argv[1], "--print-and-refresh=", &name)) {
> -               argc--;
> -               argv++;
> +       for (++argv, --argc; *argv && starts_with(*argv, "--"); ++argv, --argc) {
> +               if (skip_prefix(*argv, "--print-and-refresh=", &name))
> +                       continue;
> +               if (!strcmp(*argv, "--table")) {
> +                       table = 1;
> +               }
>         }
>
> -       if (argc == 2)
> -               cnt = strtol(argv[1], NULL, 0);
> +       if (argc == 1)
> +               cnt = strtol(argv[0], NULL, 0);
>         setup_git_directory();
>         git_config(git_default_config, NULL);
>         for (i = 0; i < cnt; i++) {
> @@ -30,6 +67,8 @@ int cmd__read_cache(int argc, const char **argv)
>                                ce_uptodate(the_index.cache[pos]) ? "" : " not");
>                         write_file(name, "%d\n", i);
>                 }
> +               if (table)
> +                       print_cache(&the_index);
>                 discard_cache();
>         }
>         return 0;
> --
> gitgitgadget
>