Hi Anoob > -----Original Message----- > From: Anoob Joseph > Sent: Thursday, April 18, 2019 9:28 AM > To: Ayuj Verma <ayve...@marvell.com>; akhil.go...@nxp.com; > arkadiuszx.kusz...@intel.com; fiona.tr...@intel.com > Cc: Shally Verma <shal...@marvell.com>; Sunila Sahu <ss...@marvell.com>; > Kanaka Durga Kotamarthy <kkotamar...@marvell.com>; Arvind Desai > <ade...@marvell.com>; dev@dpdk.org; Ayuj Verma > <ayve...@marvell.com> > Subject: RE: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering of > headers > > Hi Ayuj, Akhil, Fiona, Shally, > > I think there are couple of issues with this patch. The real issue here is the > dependency of rte_crypto_asym.h on rte_crypto_sym.h. If > rte_crypto_asym.h doesn't include all the headers it uses, every file which > includes rte_crypto_asym.h will have to include rte_crypto_asym.h's > dependencies, which is not the right practice. Keeping it alphabetical etc > comes later.
[Shally] We can change title something to "include dependencies into asym header files" , but then should I assume that You agree to this patch? > > So the patch has to be rephrased to better reflect the purpose. > > Also please see inline. > > Thanks, > Anoob > > > -----Original Message----- > > From: dev <dev-boun...@dpdk.org> On Behalf Of Ayuj Verma > > Sent: Friday, April 12, 2019 12:33 PM > > To: akhil.go...@nxp.com; arkadiuszx.kusz...@intel.com; > > fiona.tr...@intel.com > > Cc: Shally Verma <shal...@marvell.com>; Sunila Sahu > > <ss...@marvell.com>; Kanaka Durga Kotamarthy > > <kkotamar...@marvell.com>; Arvind Desai <ade...@marvell.com>; > > dev@dpdk.org; Ayuj Verma <ayve...@marvell.com> > > Subject: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering > > of headers > > > > include rte_crypto_sym.h in rte_crypto_asym.h in place of including it > > in rte_crypto.h. > > > > Signed-off-by: Ayuj Verma <ayve...@marvell.com> > > Signed-off-by: Shally Verma <shal...@marvell.com> > > > > --- > > lib/librte_cryptodev/rte_crypto.h | 1 - > > lib/librte_cryptodev/rte_crypto_asym.h | 2 ++ > > 2 files changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/lib/librte_cryptodev/rte_crypto.h > > b/lib/librte_cryptodev/rte_crypto.h > > index fd5ef3a..17dccdf 100644 > > --- a/lib/librte_cryptodev/rte_crypto.h > > +++ b/lib/librte_cryptodev/rte_crypto.h > > @@ -22,7 +22,6 @@ > > #include <rte_mempool.h> > > #include <rte_common.h> > > > > -#include "rte_crypto_sym.h" > > #include "rte_crypto_asym.h" > > [Anoob] If rte_crypto.h uses anything defined in rte_crypto_sym.h, then you > cannot remove the include. [Shally] so you mean we should do #include of sym.h in both, this and rte_crypto_asym.h file? >You can make it alphabetical, but the include has to be retained. [Shally] Sorry didn't get what you exactly mean by this? Thanks Shally > > > > > /** Crypto operation types */ > > diff --git a/lib/librte_cryptodev/rte_crypto_asym.h > > b/lib/librte_cryptodev/rte_crypto_asym.h > > index 5e43620..a55923a 100644 > > --- a/lib/librte_cryptodev/rte_crypto_asym.h > > +++ b/lib/librte_cryptodev/rte_crypto_asym.h > > @@ -25,6 +25,8 @@ > > #include <rte_mempool.h> > > #include <rte_common.h> > > > > +#include "rte_crypto_sym.h" > > + > > typedef struct rte_crypto_param_t { > > uint8_t *data; > > /**< pointer to buffer holding data */ > > -- > > 1.8.3.1