Re: [PATCH 2/8] staging: ccree: use more readable func names

From: Dan Carpenter
Date: Tue Nov 07 2017 - 05:31:08 EST


On Tue, Nov 07, 2017 at 09:39:58AM +0000, Gilad Ben-Yossef wrote:
> @@ -780,11 +766,10 @@ static inline int ssi_buffer_mgr_aead_chain_iv(
> unsigned int iv_size_to_authenc = crypto_aead_ivsize(tfm);
> unsigned int iv_ofs = GCM_BLOCK_RFC4_IV_OFFSET;
> /* Chain to given list */
> - ssi_buffer_mgr_add_buffer_entry(
> - dev, sg_data,
> - areq_ctx->gen_ctx.iv_dma_addr + iv_ofs,
> - iv_size_to_authenc, is_last,
> - &areq_ctx->assoc.mlli_nents);
> + cc_add_buffer_entry(dev, sg_data,
> + (areq_ctx->gen_ctx.iv_dma_addr + iv_ofs),
^ ^

No need to resend the patch, but you've added parenthesis here that
weren't in the original code and in other places you fixed up some long
line warnings. I'd prefer if you didn't do that because I use scripts
to review rename patches automatically and since these are not a rename
so it means I have to review them manually. Also in that patch that
Tobin complained about you had some extra comment changes and some were
related to the API but some were just reformatted to fit in 80 chars.

In this sample, you've re-indented the code a bit to align correctly.
That would be mandatory especially if the original code had aligned
correctly so that's fine. But otherwise try to be strict about the one
thing per patch.

> + iv_size_to_authenc, is_last,
> + &areq_ctx->assoc.mlli_nents);
> areq_ctx->assoc_buff_type = SSI_DMA_BUF_MLLI;
> }
>

regards,
dan carpenter