Hi Dan, Thank you for reviewing the patch set. On Tue, Nov 7, 2017 at 12:30 PM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> 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. Gilad -- Gilad Ben-Yossef 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
