Re: [PATCH 31/41] wt-status: convert two uses of EMPTY_TREE_SHA1_HEX

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 24 April 2018 at 01:39, brian m. carlson
<sandals@xxxxxxxxxxxxxxxxxxxx> wrote:
> Convert two uses of EMPTY_TREE_SHA1_HEX to use oid_to_hex_r and
> the_hash_algo to avoid a dependency on a given hash algorithm.  Use
> oid_to_hex_r in preference to oid_to_hex because the buffer needs to
> last through several function calls which might exhaust the limit of
> four static buffers.
>
> Signed-off-by: brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx>
> ---
>  wt-status.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/wt-status.c b/wt-status.c
> index 50815e5faf..857724bd60 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -600,10 +600,11 @@ static void wt_status_collect_changes_index(struct wt_status *s)
>  {
>         struct rev_info rev;
>         struct setup_revision_opt opt;
> +       char hex[GIT_MAX_HEXSZ + 1];
>
>         init_revisions(&rev, NULL);
>         memset(&opt, 0, sizeof(opt));
> -       opt.def = s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference;
> +       opt.def = s->is_initial ? oid_to_hex_r(hex, the_hash_algo->empty_tree) : s->reference;
>         setup_revisions(0, NULL, &rev, &opt);
>
>         rev.diffopt.flags.override_submodule_config = 1;
> @@ -975,13 +976,14 @@ static void wt_longstatus_print_verbose(struct wt_status *s)
>         struct setup_revision_opt opt;
>         int dirty_submodules;
>         const char *c = color(WT_STATUS_HEADER, s);
> +       char hex[GIT_MAX_HEXSZ + 1];
>
>         init_revisions(&rev, NULL);
>         rev.diffopt.flags.allow_textconv = 1;
>         rev.diffopt.ita_invisible_in_index = 1;
>
>         memset(&opt, 0, sizeof(opt));
> -       opt.def = s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference;
> +       opt.def = s->is_initial ? oid_to_hex_r(hex, the_hash_algo->empty_tree) : s->reference;
>         setup_revisions(0, NULL, &rev, &opt);

Just a thought: Maybe it would make sense to have a function
`oid_hex_empty_tree()` or similar to replace the
oid_to_hex[_r](the_hash_algo->empty_tree) idiom. It would help avoid the
buffer here, but also get rid of a few instances of code peeking into
the_hash_algo. I dunno.

I've been scanning this series semi-sloppily up to here, and left some
comments along the way.

Thank you for working on this.

Martin



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux