On Fri, Jul 26, 2019 at 02:29:48PM +0000, Pascal Van Leeuwen wrote:
> > From: linux-crypto-owner@xxxxxxxxxxxxxxx <linux-crypto-owner@xxxxxxxxxxxxxxx> On Behalf Of Antoine Tenart
> > On Fri, Jul 26, 2019 at 01:28:13PM +0000, Pascal Van Leeuwen wrote:
> > > > On Fri, Jul 05, 2019 at 08:49:23AM +0200, Pascal van Leeuwen wrote:
> > >
> > > > > @@ -62,9 +63,9 @@ static void safexcel_skcipher_token(struct safexcel_cipher_ctx *ctx, u8 *iv,
> > > > > u32 length)
> > > > > - if (ctx->mode == CONTEXT_CONTROL_CRYPTO_MODE_CBC) {
> > > > > + if (ctx->mode != CONTEXT_CONTROL_CRYPTO_MODE_ECB) {
> > > >
> > > > I think it's better for maintenance and readability to have something
> > > > like:
> > > >
> > > > if (ctx->mode == CONTEXT_CONTROL_CRYPTO_MODE_CBC ||
> > > > ctx->mode == CONTEXT_CONTROL_CRYPTO_MODE_CTR_LOAD)
> > > >
> > > Not really. I *really* want to execute that for any mode other than ECB,
> > > ECB being the *only* mode that does not require an IV (which I know
> > > for a fact, being the architect and all :-).
> > > And I don't believe a long list of modes that *do* require an IV would
> > > be more readable or easy to maintain than this single compare ...
> >
> > That's where I disagree as you need extra knowledge to be aware of this.
> > Being explicit removes any possible question one may ask. But that's a
> > small point really :)
> >
> Well, while we're disagreeing ... I disagree with your assertion that you
> would need more knowledge to know which modes do NOT need an IV
> than to know which modes DO need an IV. There's really no fundamental
> difference, it's two sides of the exact same coin ... you need that
> knowledge either way.
The point is if you look for occurrences of, let's say
CONTEXT_CONTROL_CRYPTO_MODE_CBC, to see the code path it'll be way
easier if you have direct comparisons.
> 1) This code is executed for each individual cipher call, i.e. it's in the
> critical path. Having just 1 compare-and-branch there is better for
> performance than having many.
Not sure about what the impact really is.
> 2) Generally, all else being equal, having less code is easier to maintain
> than having more code.
That really depends, having readable code is easier to maintain :)
> 4) If there is anything unclear about an otherwise fine code construct,
> then you clarify it by adding a comment, not by rewriting it to be inefficient
> and redundant ;-)
Fair point.
Thanks,
Antoine
--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
