linux-crypto.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Herbert Xu <herbert@gondor.apana.org.au>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	"H . Peter Anvin" <hpa@zytor.com>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	Vasily Gorbik <gor@linux.ibm.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	linux-crypto@vger.kernel.org, x86@kernel.org,
	linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/6] crypto: sha256 - Move lib/sha256.c to lib/crypto
Date: Sat, 17 Aug 2019 10:28:04 +0200	[thread overview]
Message-ID: <909d255d-a648-13b5-100f-fe67be547961@redhat.com> (raw)
In-Reply-To: <20190817051942.GB8209@sol.localdomain>

Hi,

On 17-08-19 07:19, Eric Biggers wrote:
> On Fri, Aug 16, 2019 at 11:16:08PM +0200, Hans de Goede wrote:
>> diff --git a/include/linux/sha256.h b/include/crypto/sha256.h
>> similarity index 100%
>> rename from include/linux/sha256.h
>> rename to include/crypto/sha256.h
> 
> <crypto/sha.h> already has the declarations for both SHA-1 and SHA-2, including
> SHA-256.  So I'm not sure a separate sha256.h is appropriate.  How about putting
> these declarations in <crypto/sha.h>?

The problems with that is that the sha256_init, etc. names are quite generic
and they have not been reserved before, so a lot of the crypto hw-accel
drivers use them, for private file-local (static) code, e.g.:

[hans@shalem linux]$ ack -l sha256_init
include/crypto/sha256.h
drivers/crypto/marvell/hash.c
drivers/crypto/ccp/ccp-ops.c
drivers/crypto/nx/nx-sha256.c
drivers/crypto/ux500/hash/hash_core.c
drivers/crypto/inside-secure/safexcel_hash.c
drivers/crypto/chelsio/chcr_algo.h
drivers/crypto/stm32/stm32-hash.c
drivers/crypto/omap-sham.c
drivers/crypto/padlock-sha.c
drivers/crypto/n2_core.c
drivers/crypto/atmel-aes.c
drivers/crypto/axis/artpec6_crypto.c
drivers/crypto/mediatek/mtk-sha.c
drivers/crypto/qat/qat_common/qat_algs.c
drivers/crypto/img-hash.c
drivers/crypto/ccree/cc_hash.c
lib/crypto/sha256.c
arch/powerpc/crypto/sha256-spe-glue.c
arch/mips/cavium-octeon/crypto/octeon-sha256.c
arch/x86/purgatory/purgatory.c
arch/s390/crypto/sha256_s390.c
arch/s390/purgatory/purgatory.c

(in case you do not know ack is a smarter grep, which skips .o files, etc.)

All these do include crypto/sha.h and putting the stuff which is in what
was linux/sha256.h into crypto/sha.h leads to name collisions which causes
more churn then I would like this series to cause.

I guess we could do a cleanup afterwards, with one patch per file above
to fix the name collision issue, and then merge the 2 headers. I do not
want to do that for this series, as I want to keep this series as KISS
as possible since it is messing with somewhat sensitive stuff.

And TBH I even wonder if a follow-up series is worth the churn...

Regards,

Hans


  reply	other threads:[~2019-08-17  8:28 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-16 21:16 [PATCH 0/6] crypto: sha256 - Merge 2 separate C implementations into 1, put into separate library Hans de Goede
2019-08-16 21:16 ` [PATCH 1/6] crypto: sha256 - Fix some coding style issues Hans de Goede
2019-08-16 21:16 ` [PATCH 2/6] crypto: sha256_generic " Hans de Goede
2019-08-16 21:16 ` [PATCH 3/6] crypto: sha256 - Move lib/sha256.c to lib/crypto Hans de Goede
2019-08-17  5:19   ` Eric Biggers
2019-08-17  8:28     ` Hans de Goede [this message]
2019-08-18 15:54       ` Eric Biggers
2019-08-18 16:08         ` Hans de Goede
2019-08-16 21:16 ` [PATCH 4/6] crypto: sha256 - Use get_unaligned_be32 to get input, memzero_explicit Hans de Goede
2019-08-17  5:37   ` Eric Biggers
2019-08-16 21:16 ` [PATCH 5/6] crypto: sha256 - Make lib/crypto/sha256.c suitable for generic use Hans de Goede
2019-08-16 21:16 ` [PATCH 6/6] crypto: sha256_generic - Use sha256_transform from generic sha256 lib Hans de Goede
2019-08-17  5:13   ` Eric Biggers
2019-08-17  5:35     ` Eric Biggers
2019-08-17 12:18       ` Hans de Goede

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=909d255d-a648-13b5-100f-fe67be547961@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=borntraeger@de.ibm.com \
    --cc=bp@alien8.de \
    --cc=gor@linux.ibm.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=hpa@zytor.com \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).