Received: by 2002:ab2:3319:0:b0:1ef:7a0f:c32d with SMTP id i25csp391428lqc; Thu, 7 Mar 2024 23:56:57 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCWl033AlcdHGaqkaLX+Q38EEbiJpmUMIw8fEfYEn2w3KmKCvlaceo55WbxPcCqDJyeu3l2Rsj1BDEXrxYb/E/NEgtTu/zo/dVC+dBQtPQ== X-Google-Smtp-Source: AGHT+IFvVIIkTjwF6p5Gis0kX9yfxkob9LPRtV2cqoa2THe7Ps2depR+nApf0gvETVQVnKWzRPOQ X-Received: by 2002:a17:906:b294:b0:a45:bf15:640b with SMTP id q20-20020a170906b29400b00a45bf15640bmr4552340ejz.70.1709884617765; Thu, 07 Mar 2024 23:56:57 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1709884617; cv=pass; d=google.com; s=arc-20160816; b=kbZuWBYjdnBBZapK2MYIC/NjVgXGKJZhwUPrVNan86gi+BNySFC7SODN7PO56YazLR Kve0ST2hkLcfamowY90iJaFgxnHQQnS5OF1y6WJIjKpbVdTqn8tScd8uD4Ik8RnQEEua /UbWLcYlgN/atAiJkD7CdGYzAHjGEx2LYHwVte5umBw65DOfsWw524qZ4c8nY8+Jd748 /8S7n9nqu/d1mcjFchxEfJ+/Bfx5LSCeF0cGnCBdFPPkJs7SofUULAunnL1Y0rKnr203 3wdTBhUC/t3IkHcxOfsHquGP6DApsUUsPkWPesiuzkVFxD0WS5H1itUKYIGUGTaiszqt xz9Q== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=in-reply-to:content-disposition:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:references:message-id:subject:cc :to:from:date; bh=PtowWNqHHiVWHlUlOnK3Z9mXwPOe7lw3IgNmojzNaFs=; fh=xHyPvxm2qUH0RTXf3WjkJn/1jWA55EsE0feGd+LgC9M=; b=Mdl5Os6hZ91FkS7vl0UUzTItacJnnv0NXAtY2pHrD7cQB92lVwJURjxNBksR5GydmV vrV4mvHIbJ7oU2g7l2024lKaRpZ+XnFeVGADlbpJJBL7giyRbKAX+ZQPVzJXpK1Zsalh hyDjs99aFi2rmbJ5R6lLVgpdTbi2lNM4/Wpkd7fmYn7FsopVBVEjvoLUsYJxh7ReTwyJ 0eoacIKBAvPe8FlzGPTohvGzsKgldAW4or1aNgHcfFF+stpETdj43Q3b9FXTkc+bXL3Y xrBLyFIgTvLO3oLJ5focgLvjTQaXn7JcvwRWYrFGQSFKtUSQXzxrOCk3LlfmrZbQEJ+K +zLg==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; arc=pass (i=1 spf=pass spfdomain=altlinux.org); spf=pass (google.com: domain of linux-kernel+bounces-96612-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) smtp.mailfrom="linux-kernel+bounces-96612-linux.lists.archive=gmail.com@vger.kernel.org" Return-Path: Received: from am.mirrors.kernel.org (am.mirrors.kernel.org. [2604:1380:4601:e00::3]) by mx.google.com with ESMTPS id x10-20020a1709065aca00b00a455c1335fesi4431726ejs.676.2024.03.07.23.56.57 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Mar 2024 23:56:57 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-96612-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3; Authentication-Results: mx.google.com; arc=pass (i=1 spf=pass spfdomain=altlinux.org); spf=pass (google.com: domain of linux-kernel+bounces-96612-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) smtp.mailfrom="linux-kernel+bounces-96612-linux.lists.archive=gmail.com@vger.kernel.org" Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id E8EF81F23643 for ; Fri, 8 Mar 2024 07:56:55 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id E03EA4F8BB; Fri, 8 Mar 2024 07:56:39 +0000 (UTC) Received: from vmicros1.altlinux.org (vmicros1.altlinux.org [194.107.17.57]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 7E4B74EB43; Fri, 8 Mar 2024 07:56:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=194.107.17.57 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709884599; cv=none; b=ePA0xhn/vcmK74k8HXf6wL6TaD78Ieki/OfgG/xdOj8EUzETjCzxK9NwsSssqICTEzPrJIfc20ZEHXFFL/IBogB4sewCyM0uStPBOWtIhBMD9vnQ4JUdAL6neVtjj4XxJWi7vU1h+ZqyKnyQCblEqLR+IXFhFYJzvQZfL7tdC3k= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709884599; c=relaxed/simple; bh=77y5yz7tvTPJlsDrybYtETDg+H02RhwsEJCmIwEOSqg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=HpNkUhcJ8LB08+3IvPLjX1H8I3mqtyrtIWuuYqALUyj6i9RciqIJOWmQYYgcllMfATxhnVkS3/dcSjAOICYdXL9FHvVWiy1NImBIQi/enTHqjQswNaIl0sD94Ov08N+uGwkBUOxXFeQEEL3zKAC6b3PAp7GFF1etHacWIqgNCxY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=altlinux.org; spf=pass smtp.mailfrom=altlinux.org; arc=none smtp.client-ip=194.107.17.57 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=altlinux.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=altlinux.org Received: from imap.altlinux.org (imap.altlinux.org [194.107.17.38]) by vmicros1.altlinux.org (Postfix) with ESMTP id EEBCD72C980; Fri, 8 Mar 2024 10:48:42 +0300 (MSK) Received: from altlinux.org (sole.flsd.net [185.75.180.6]) by imap.altlinux.org (Postfix) with ESMTPSA id DFA9436D0168; Fri, 8 Mar 2024 10:48:42 +0300 (MSK) Date: Fri, 8 Mar 2024 10:48:42 +0300 From: Vitaly Chikunov To: Jarkko Sakkinen , Stefan Berger Cc: keyrings@vger.kernel.org, linux-crypto@vger.kernel.org, herbert@gondor.apana.org.au, davem@davemloft.net, linux-kernel@vger.kernel.org, saulo.alessandre@tse.jus.br, lukas@wunner.de Subject: Re: [PATCH v5 09/12] crypto: ecdsa - Rename keylen to bufsize where necessary Message-ID: <20240308074842.g25bsphtbwdzzo4l@altlinux.org> References: <20240306222257.979304-1-stefanb@linux.ibm.com> <20240306222257.979304-10-stefanb@linux.ibm.com> <571aa199-00cc-4153-9424-0012d20dc6f6@linux.ibm.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=koi8-r Content-Disposition: inline In-Reply-To: <571aa199-00cc-4153-9424-0012d20dc6f6@linux.ibm.com> Jarkko, On Thu, Mar 07, 2024 at 02:20:12PM -0500, Stefan Berger wrote: > On 3/7/24 14:13, Jarkko Sakkinen wrote: > > On Thu Mar 7, 2024 at 12:22 AM EET, Stefan Berger wrote: > > > In some cases the name keylen does not reflect the purpose of the variable > > > anymore once NIST P521 is used but it is the size of the buffer. There- > > > for, rename keylen to bufsize where appropriate. > > > > > > Signed-off-by: Stefan Berger > > > Tested-by: Lukas Wunner > > > --- > > > crypto/ecdsa.c | 12 ++++++------ > > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > > > diff --git a/crypto/ecdsa.c b/crypto/ecdsa.c > > > index 4daefb40c37a..4e847b59622a 100644 > > > --- a/crypto/ecdsa.c > > > +++ b/crypto/ecdsa.c > > > @@ -35,8 +35,8 @@ struct ecdsa_signature_ctx { > > > static int ecdsa_get_signature_rs(u64 *dest, size_t hdrlen, unsigned char tag, > > > const void *value, size_t vlen, unsigned int ndigits) > > > { > > > - size_t keylen = ndigits * sizeof(u64); > > > > nit: still don't get why "* sizeof(u64)" would ever be more readable > > thean "* 8". > > Because existing code in crypto uses sizeof(u64) when converting ndigits to > number of bytes and '8' is not used for converting to bytes. Do we need to > change this now ? No, I think it's better to conform to existing code. `sizeof(u64)` is easily read as `8` by reviewers, but just `8` will require inline comments because it's magic number isn't it? So this will not even decrease number of letters. `sizeof(u64)` is self-documenting code and you don't even need to interpret it as `8` for review as you don't need number from any sizeof(struct ..). Also, possible we need to calculate number of bits in the big number, so this would become `* 8 * 8`, in that case how would you distinguish omission of one `* 8` by a typo. Overall it's quite common in the whole tree linux/torvalds$ git grep -e '\* sizeof(u64)' -e 'sizeof(u64) \*' | wc -l 551 So this is perhaps acceptable and depends on point of view. crypto subsystem coders seems to prefer not to save on letters and type `sizeof(u64)`. Thanks, > > # grep -rI ndigits crypto/ | grep sizeof\(u64\) > crypto/ecrdsa.c: unsigned int ndigits = req->dst_len / sizeof(u64); > crypto/ecrdsa.c: req->dst_len != ctx->curve->g.ndigits * > sizeof(u64) || > crypto/ecrdsa.c: vli_from_be64(r, sig + ndigits * sizeof(u64), > ndigits); > crypto/ecrdsa.c: ctx->curve->g.ndigits * sizeof(u64) != > ctx->digest_len) > crypto/ecrdsa.c: ctx->key_len != ctx->curve->g.ndigits * > sizeof(u64) * 2) > crypto/ecrdsa.c: ndigits = ctx->key_len / sizeof(u64) / 2; > crypto/ecrdsa.c: vli_from_le64(ctx->pub_key.y, ctx->key + ndigits * > sizeof(u64), > crypto/ecrdsa.c: return ctx->pub_key.ndigits * sizeof(u64); > crypto/ecdh.c: params.key_size > sizeof(u64) * ctx->ndigits) > crypto/ecc.c: size_t len = ndigits * sizeof(u64); > crypto/ecc.c: num_bits = sizeof(u64) * ndigits * 8 + 1; > crypto/ecdsa.c: size_t bufsize = ndigits * sizeof(u64); > crypto/ecdsa.c: size_t bufsize = ctx->curve->g.ndigits * sizeof(u64); > crypto/ecdsa.c: ndigits = DIV_ROUND_UP(digitlen, sizeof(u64)); > > Stefan > > > > > BR, Jarkko