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

[brian m. carlson]

On Tue, Apr 24, 2018 at 12:03:35PM +0200, Martin Ågren wrote:
> 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.

At first I wasn't going to include this change in the series, but then I
thought about what a good idea it was and decided to redo the
corresponding patches.  So I hope to have a v2 out tomorrow with this
change (and the rest of them) in it.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

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

[Martin Ågren]

On 24 April 2018 at 01:39, brian m. carlson
 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 
> ---
>  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(, NULL);
> memset(, 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.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(, NULL);
> rev.diffopt.flags.allow_textconv = 1;
> rev.diffopt.ita_invisible_in_index = 1;
>
> memset(, 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, , );

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