From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.kernel.org ([198.145.29.99]:41910 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726089AbfHRPy4 (ORCPT ); Sun, 18 Aug 2019 11:54:56 -0400 Date: Sun, 18 Aug 2019 08:54:53 -0700 From: Eric Biggers Subject: Re: [PATCH 3/6] crypto: sha256 - Move lib/sha256.c to lib/crypto Message-ID: <20190818155453.GC1118@sol.localdomain> References: <20190816211611.2568-1-hdegoede@redhat.com> <20190816211611.2568-4-hdegoede@redhat.com> <20190817051942.GB8209@sol.localdomain> <909d255d-a648-13b5-100f-fe67be547961@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <909d255d-a648-13b5-100f-fe67be547961@redhat.com> Sender: linux-s390-owner@vger.kernel.org List-ID: To: Hans de Goede Cc: Herbert Xu , Thomas Gleixner , Ingo Molnar , Borislav Petkov , "H . Peter Anvin" , Heiko Carstens , Vasily Gorbik , Christian Borntraeger , Ard Biesheuvel , linux-crypto@vger.kernel.org, x86@kernel.org, linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org On Sat, Aug 17, 2019 at 10:28:04AM +0200, Hans de Goede wrote: > 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 > > > > 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 ? > > 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.) You need to match at word boundaries to avoid matching on ${foo}_sha256_init(). So it's actually a somewhat shorter list: $ git grep -l -E '\' arch/arm/crypto/sha256_glue.c arch/arm/crypto/sha256_neon_glue.c arch/arm64/crypto/sha256-glue.c arch/s390/crypto/sha256_s390.c arch/s390/purgatory/purgatory.c arch/x86/crypto/sha256_ssse3_glue.c arch/x86/purgatory/purgatory.c crypto/sha256_generic.c drivers/crypto/ccree/cc_hash.c drivers/crypto/chelsio/chcr_algo.h drivers/crypto/n2_core.c include/linux/sha256.h lib/sha256.c 5 of these are already edited by this patchset, so that leaves only 8 files. > > 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... > I think it should be done; the same was done when introducing the AES library. But I'm okay with it being done later, if you want to keep this patchset shorter. - Eric