[PATCH 2/8] staging: ccree: use more readable func names
gilad at benyossef.com
Thu Nov 9 06:26:21 UTC 2017
Thank you for reviewing the patch set.
On Tue, Nov 7, 2017 at 12:30 PM, Dan Carpenter <dan.carpenter at oracle.com> wrote:
> 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;
> > }
Noted. I have changed the patch description to at least reflect what it is
doing more accurately (i.e. more than just func name switching).
Thanks again for the review.
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
More information about the devel