On Mon, Oct 19, 2015 at 04:25:07PM +0100, Russell King - ARM Linux wrote:
> On Mon, Oct 19, 2015 at 03:04:51PM +0000, Jason Cooper wrote:
> > Hey Russell,
> >
> > On Sun, Oct 18, 2015 at 05:23:40PM +0100, Russell King wrote:
> > > static int mv_cesa_ahash_init(struct ahash_request *req,
> > > - struct mv_cesa_op_ctx *tmpl)
> > > + struct mv_cesa_op_ctx *tmpl, bool algo_le)
> >
> > nit: Would it make more sense the do bool algo_endian, and then ...
>
> I don't like "bool"s that don't self-document what they mean.
> What does "if (algo_endian)" mean? It's pretty clear what
That's where I would go with an enum. "if (algo_endian ==
ALGO_ENDIAN_LITTLE) ..."
> "if (algo_le)" means in the context of endianness, though I'll
> give you that "if (algo_little_endian)" would be a lot better.
Right, it's really a question of where do we want readability? I was
focusing on the call site, since the context isn't there for newcomers to
easily grok 'true'.
> > > @@ -861,7 +862,7 @@ static int mv_cesa_md5_init(struct ahash_request *req)
> > >
> > > mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_MD5);
> > >
> > > - mv_cesa_ahash_init(req, &tmpl);
> > > + mv_cesa_ahash_init(req, &tmpl, true);
> >
> > mv_cesa_ahash_init(req, &tmpl, ALGO_ENDIAN_LITTLE);
> >
> > 'true' doesn't seem as obvious. But again, nit-picky.
>
> I did think about:
>
> enum {
> ALGO_LITTLE_ENDIAN,
> ALGO_BIG_ENDIAN,
> };
>
> and passing an int algo_endian around, but that seemed to be like too
> much bloat... though if you want to insist, I could make that change.
Like I said, it's a nit. Either a self-documenting bool, or an enum
will work.
thx,
Jason.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
