Linux-Crypto Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] crypto: x86/curve25519 - Remove unused carry variables
@ 2020-07-23  7:50 Herbert Xu
  2020-07-23 10:05 ` Jason A. Donenfeld
  0 siblings, 1 reply; 5+ messages in thread
From: Herbert Xu @ 2020-07-23  7:50 UTC (permalink / raw)
  To: Linux Crypto Mailing List, Jason A. Donenfeld

The carry variables are assigned but never used, which upsets
the compiler.  This patch removes them.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/arch/x86/crypto/curve25519-x86_64.c b/arch/x86/crypto/curve25519-x86_64.c
index 8a17621f7d3a..8acbb6584a37 100644
--- a/arch/x86/crypto/curve25519-x86_64.c
+++ b/arch/x86/crypto/curve25519-x86_64.c
@@ -948,10 +948,8 @@ static void store_felem(u64 *b, u64 *f)
 {
 	u64 f30 = f[3U];
 	u64 top_bit0 = f30 >> (u32)63U;
-	u64 carry0;
 	u64 f31;
 	u64 top_bit;
-	u64 carry;
 	u64 f0;
 	u64 f1;
 	u64 f2;
@@ -970,11 +968,11 @@ static void store_felem(u64 *b, u64 *f)
 	u64 o2;
 	u64 o3;
 	f[3U] = f30 & (u64)0x7fffffffffffffffU;
-	carry0 = add_scalar(f, f, (u64)19U * top_bit0);
+	add_scalar(f, f, (u64)19U * top_bit0);
 	f31 = f[3U];
 	top_bit = f31 >> (u32)63U;
 	f[3U] = f31 & (u64)0x7fffffffffffffffU;
-	carry = add_scalar(f, f, (u64)19U * top_bit);
+	add_scalar(f, f, (u64)19U * top_bit);
 	f0 = f[0U];
 	f1 = f[1U];
 	f2 = f[2U];
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] crypto: x86/curve25519 - Remove unused carry variables
  2020-07-23  7:50 [PATCH] crypto: x86/curve25519 - Remove unused carry variables Herbert Xu
@ 2020-07-23 10:05 ` Jason A. Donenfeld
  2020-07-27 15:08   ` Karthik Bhargavan
  0 siblings, 1 reply; 5+ messages in thread
From: Jason A. Donenfeld @ 2020-07-23 10:05 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Linux Crypto Mailing List, Karthik Bhargavan

Hi Herbert,

On Thu, Jul 23, 2020 at 9:51 AM Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> The carry variables are assigned but never used, which upsets
> the compiler.  This patch removes them.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> diff --git a/arch/x86/crypto/curve25519-x86_64.c b/arch/x86/crypto/curve25519-x86_64.c
> index 8a17621f7d3a..8acbb6584a37 100644
> --- a/arch/x86/crypto/curve25519-x86_64.c
> +++ b/arch/x86/crypto/curve25519-x86_64.c
> @@ -948,10 +948,8 @@ static void store_felem(u64 *b, u64 *f)
>  {
>         u64 f30 = f[3U];
>         u64 top_bit0 = f30 >> (u32)63U;
> -       u64 carry0;
>         u64 f31;
>         u64 top_bit;
> -       u64 carry;
>         u64 f0;
>         u64 f1;
>         u64 f2;
> @@ -970,11 +968,11 @@ static void store_felem(u64 *b, u64 *f)
>         u64 o2;
>         u64 o3;
>         f[3U] = f30 & (u64)0x7fffffffffffffffU;
> -       carry0 = add_scalar(f, f, (u64)19U * top_bit0);
> +       add_scalar(f, f, (u64)19U * top_bit0);
>         f31 = f[3U];
>         top_bit = f31 >> (u32)63U;
>         f[3U] = f31 & (u64)0x7fffffffffffffffU;
> -       carry = add_scalar(f, f, (u64)19U * top_bit);
> +       add_scalar(f, f, (u64)19U * top_bit);
>         f0 = f[0U];
>         f1 = f[1U];
>         f2 = f[2U];
> --

That seems obvious and reasonable, and so I'm inclined to ack this,
but I first wanted to give Karthik (CC'd) a chance to chime in here,
as it's his HACL* project that's responsible, and he might have some
curious insight.

Jason

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] crypto: x86/curve25519 - Remove unused carry variables
  2020-07-23 10:05 ` Jason A. Donenfeld
@ 2020-07-27 15:08   ` Karthik Bhargavan
  2020-07-27 15:10     ` Karthik Bhargavan
  2020-07-27 15:10     ` Jason A. Donenfeld
  0 siblings, 2 replies; 5+ messages in thread
From: Karthik Bhargavan @ 2020-07-27 15:08 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: Herbert Xu, Linux Crypto Mailing List

Removing unused variables is harmless. (GCC would do this automaticelly.)
So this change seems fine.

-Karthik

> On 23 Jul 2020, at 12:05, Jason A. Donenfeld <jason@zx2c4.com> wrote:
> 
> Hi Herbert,
> 
> On Thu, Jul 23, 2020 at 9:51 AM Herbert Xu <herbert@gondor.apana.org.au> wrote:
>> 
>> The carry variables are assigned but never used, which upsets
>> the compiler.  This patch removes them.
>> 
>> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>> 
>> diff --git a/arch/x86/crypto/curve25519-x86_64.c b/arch/x86/crypto/curve25519-x86_64.c
>> index 8a17621f7d3a..8acbb6584a37 100644
>> --- a/arch/x86/crypto/curve25519-x86_64.c
>> +++ b/arch/x86/crypto/curve25519-x86_64.c
>> @@ -948,10 +948,8 @@ static void store_felem(u64 *b, u64 *f)
>> {
>>        u64 f30 = f[3U];
>>        u64 top_bit0 = f30 >> (u32)63U;
>> -       u64 carry0;
>>        u64 f31;
>>        u64 top_bit;
>> -       u64 carry;
>>        u64 f0;
>>        u64 f1;
>>        u64 f2;
>> @@ -970,11 +968,11 @@ static void store_felem(u64 *b, u64 *f)
>>        u64 o2;
>>        u64 o3;
>>        f[3U] = f30 & (u64)0x7fffffffffffffffU;
>> -       carry0 = add_scalar(f, f, (u64)19U * top_bit0);
>> +       add_scalar(f, f, (u64)19U * top_bit0);
>>        f31 = f[3U];
>>        top_bit = f31 >> (u32)63U;
>>        f[3U] = f31 & (u64)0x7fffffffffffffffU;
>> -       carry = add_scalar(f, f, (u64)19U * top_bit);
>> +       add_scalar(f, f, (u64)19U * top_bit);
>>        f0 = f[0U];
>>        f1 = f[1U];
>>        f2 = f[2U];
>> --
> 
> That seems obvious and reasonable, and so I'm inclined to ack this,
> but I first wanted to give Karthik (CC'd) a chance to chime in here,
> as it's his HACL* project that's responsible, and he might have some
> curious insight.
> 
> Jason


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] crypto: x86/curve25519 - Remove unused carry variables
  2020-07-27 15:08   ` Karthik Bhargavan
@ 2020-07-27 15:10     ` Karthik Bhargavan
  2020-07-27 15:10     ` Jason A. Donenfeld
  1 sibling, 0 replies; 5+ messages in thread
From: Karthik Bhargavan @ 2020-07-27 15:10 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: Herbert Xu, Linux Crypto Mailing List

Reviewed-by: Karthikeyan Bhargavan <karthik.bhargavan@gmail.com>

> On 27 Jul 2020, at 17:08, Karthik Bhargavan <karthikeyan.bhargavan@inria.fr> wrote:
> 
> Removing unused variables is harmless. (GCC would do this automaticelly.)
> So this change seems fine.
> 
> -Karthik
> 
>> On 23 Jul 2020, at 12:05, Jason A. Donenfeld <jason@zx2c4.com> wrote:
>> 
>> Hi Herbert,
>> 
>> On Thu, Jul 23, 2020 at 9:51 AM Herbert Xu <herbert@gondor.apana.org.au> wrote:
>>> 
>>> The carry variables are assigned but never used, which upsets
>>> the compiler.  This patch removes them.
>>> 
>>> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>>> 
>>> diff --git a/arch/x86/crypto/curve25519-x86_64.c b/arch/x86/crypto/curve25519-x86_64.c
>>> index 8a17621f7d3a..8acbb6584a37 100644
>>> --- a/arch/x86/crypto/curve25519-x86_64.c
>>> +++ b/arch/x86/crypto/curve25519-x86_64.c
>>> @@ -948,10 +948,8 @@ static void store_felem(u64 *b, u64 *f)
>>> {
>>>       u64 f30 = f[3U];
>>>       u64 top_bit0 = f30 >> (u32)63U;
>>> -       u64 carry0;
>>>       u64 f31;
>>>       u64 top_bit;
>>> -       u64 carry;
>>>       u64 f0;
>>>       u64 f1;
>>>       u64 f2;
>>> @@ -970,11 +968,11 @@ static void store_felem(u64 *b, u64 *f)
>>>       u64 o2;
>>>       u64 o3;
>>>       f[3U] = f30 & (u64)0x7fffffffffffffffU;
>>> -       carry0 = add_scalar(f, f, (u64)19U * top_bit0);
>>> +       add_scalar(f, f, (u64)19U * top_bit0);
>>>       f31 = f[3U];
>>>       top_bit = f31 >> (u32)63U;
>>>       f[3U] = f31 & (u64)0x7fffffffffffffffU;
>>> -       carry = add_scalar(f, f, (u64)19U * top_bit);
>>> +       add_scalar(f, f, (u64)19U * top_bit);
>>>       f0 = f[0U];
>>>       f1 = f[1U];
>>>       f2 = f[2U];
>>> --
>> 
>> That seems obvious and reasonable, and so I'm inclined to ack this,
>> but I first wanted to give Karthik (CC'd) a chance to chime in here,
>> as it's his HACL* project that's responsible, and he might have some
>> curious insight.
>> 
>> Jason
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] crypto: x86/curve25519 - Remove unused carry variables
  2020-07-27 15:08   ` Karthik Bhargavan
  2020-07-27 15:10     ` Karthik Bhargavan
@ 2020-07-27 15:10     ` Jason A. Donenfeld
  1 sibling, 0 replies; 5+ messages in thread
From: Jason A. Donenfeld @ 2020-07-27 15:10 UTC (permalink / raw)
  To: Karthik Bhargavan; +Cc: Herbert Xu, Linux Crypto Mailing List

On Mon, Jul 27, 2020 at 5:08 PM Karthik Bhargavan
<karthikeyan.bhargavan@inria.fr> wrote:
>
> Removing unused variables is harmless. (GCC would do this automaticelly.)
> So this change seems fine.

Thanks for confirming.  Hopefully we can get that change upstream in
HACL* too, so that the code comes out the same.

Acked-by: Jason A. Donenfeld <Jason@zx2c4.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-23  7:50 [PATCH] crypto: x86/curve25519 - Remove unused carry variables Herbert Xu
2020-07-23 10:05 ` Jason A. Donenfeld
2020-07-27 15:08   ` Karthik Bhargavan
2020-07-27 15:10     ` Karthik Bhargavan
2020-07-27 15:10     ` Jason A. Donenfeld

Linux-Crypto Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-crypto/0 linux-crypto/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-crypto linux-crypto/ https://lore.kernel.org/linux-crypto \
		linux-crypto@vger.kernel.org
	public-inbox-index linux-crypto

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-crypto


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git