All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <johannes.schindelin@gmx.de>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/7] Makefile: optionally compile with both SHA1DC and SHA1_OPENSSL
Date: Thu, 30 Mar 2017 09:47:13 -0700	[thread overview]
Message-ID: <xmqqvaqq67am.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <xmqq37du7n9p.fsf@gitster.mtv.corp.google.com> (Junio C. Hamano's message of "Thu, 30 Mar 2017 09:16:50 -0700")

Junio C Hamano <gitster@pobox.com> writes:

> ...
> The use of union is a good ingredient for a solution.  I would have
> chosen to do this slightly differently if I were doing it.
>
>         typedef struct {
>                 int safe;
>                 union {
>                         SHA1_CTX_SAFE safe;
>                         SHA1_CTX_FAST fast;
>                 } u;
>         } git_SHA_CTX;
>
>         void git_SHA1_Init(git_SHA_CTX *ctx, int safe);
> 	void git_SHA1_Update(git_SHA_CTX *ctx, const void *, unsigned long);
> 	git_SHA1_Final(uchar [20], git_SHA_CTX *ctx);
>
> where SHA1_CTX_FAST may be chosen from the Makefile just like we
> currently choose platform_SHA_CTX.  SHA1_CTX_SAFE could also be made
> configurable but it may be OK to hardcode it to refer to SHA1_CTX of
> DC's.
>
> As you already know, I am assuming that each codepath pretty much
> knows if it needs safe or fast one (e.g. the one used in csum-file.c
> knows it does not have to), so each git_SHA_CTX is told which one to
> use when it gets initialized.

And if we wanted to declare "git add" is always safe, we could still
do

    int sha1_safety_global_override = -1; /* unspecified */

    void git_SHA1_Init(git_SHA_CTX *ctx, int safe)
    {
	if (sha1_safety_global_override >= 0)
	    ctx->safe = sha1_safety_global_override;
        else
	    ctx->safe = safe;

	if (ctx->safe)
	    SHA1DCInit(&(ctx->u.safe));
	else
	    platform_SHA1_Init(&(ctx->u.fast));
   }

and then have cmd_add() in builtin/add.c to flip that global
override bit to say "this does not have to be safe".  I personally
do not think it is a good idea, but I am showing that it is still
doable.  

And as long as assignment to sha1_safety_global_override is done in
a thread-friendly way, such a scheme would be more thread-friendly
as a whole compared to the "toggle_sha1dc()" approach where each CTX
instance does not know which side of the union it is being used us
(which, if mixed-up, of course would lead to a funny behaviour).


  reply	other threads:[~2017-03-30 16:47 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-24 23:24 [PATCH 0/7] PREVIEW: Introduce DC_AND_OPENSSL_SHA1 make flag Johannes Schindelin
2017-03-24 23:24 ` [PATCH 1/7] sha1dc: safeguard against outside definitions of BIGENDIAN Johannes Schindelin
2017-03-24 23:24 ` [PATCH 2/7] Makefile: optionally compile with both SHA1DC and SHA1_OPENSSL Johannes Schindelin
2017-03-25 19:51   ` Ævar Arnfjörð Bjarmason
2017-03-30 16:16   ` Junio C Hamano
2017-03-30 16:47     ` Junio C Hamano [this message]
2017-04-18 11:28     ` Johannes Schindelin
2017-03-24 23:24 ` [PATCH 3/7] config: add the core.enablesha1dc setting Johannes Schindelin
2017-03-24 23:25 ` [PATCH 4/7] t0013: do not skip the entire file wholesale without DC_SHA1 Johannes Schindelin
2017-03-24 23:25 ` [PATCH 5/7] t0013: test DC_AND_OPENSSL_SHA1, too Johannes Schindelin
2017-03-24 23:28 ` [PATCH 6/7] mingw: enable DC_AND_OPENSSL_SHA1 by default Johannes Schindelin
2017-03-24 23:28 ` [PATCH 7/7] p0013: new test to compare SHA1DC vs OpenSSL Johannes Schindelin
2017-03-25  6:37 ` [PATCH 0/7] PREVIEW: Introduce DC_AND_OPENSSL_SHA1 make flag Junio C Hamano
2017-03-25 16:58   ` Junio C Hamano
2017-03-26  6:18   ` Jeff King
2017-03-26 23:16     ` Junio C Hamano
2017-03-27  1:11       ` Jeff King
2017-03-27  6:07         ` Junio C Hamano
2017-03-27  7:09           ` Jeff King
2017-03-27 17:15             ` Junio C Hamano
2017-03-29 20:02   ` Johannes Schindelin
2017-03-30  0:31     ` Junio C Hamano
2017-04-18 11:30       ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqvaqq67am.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.