From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 354AF11A4 for ; Wed, 14 Nov 2018 09:35:22 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Nov 2018 00:35:21 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,231,1539673200"; d="scan'208";a="106126852" Received: from irsmsx104.ger.corp.intel.com ([163.33.3.159]) by fmsmga004.fm.intel.com with ESMTP; 14 Nov 2018 00:35:19 -0800 Received: from irsmsx106.ger.corp.intel.com ([169.254.8.8]) by IRSMSX104.ger.corp.intel.com ([169.254.5.131]) with mapi id 14.03.0415.000; Wed, 14 Nov 2018 08:35:18 +0000 From: "Ananyev, Konstantin" To: "Trahe, Fiona" , "dev@dpdk.org" CC: "De Lara Guarch, Pablo" , Akhil Goyal , "Doherty, Declan" , "Ravi Kumar" , Jerin Jacob , "Zhang, Roy Fan" , Tomasz Duszynski , Hemant Agrawal , Natalie Samsonov , Dmitri Epshtein , Jay Zhou , "Zhang, Roy Fan" Thread-Topic: [RFC] cryptodev: proposed changes in rte_cryptodev_sym_session Thread-Index: AQHUO9Lg2zx23KOBQ0qIus6aAigF26VNHgOAgAFaq7CAAHaeAIAAgWUg Date: Wed, 14 Nov 2018 08:35:17 +0000 Message-ID: <2601191342CEEE43887BDE71AB977258010CEB62FB@IRSMSX106.ger.corp.intel.com> References: <1535132906-5167-1-git-send-email-konstantin.ananyev@intel.com> <348A99DA5F5B7549AA880327E580B43589676DA2@IRSMSX101.ger.corp.intel.com> <2601191342CEEE43887BDE71AB977258010CE4AB41@IRSMSX106.ger.corp.intel.com> <348A99DA5F5B7549AA880327E580B43589677E12@IRSMSX101.ger.corp.intel.com> In-Reply-To: <348A99DA5F5B7549AA880327E580B43589677E12@IRSMSX101.ger.corp.intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNzYyYmEwZjAtMjg1NC00MGY1LTg2NmQtNDNiNjczMzg1NDVmIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiOXBaRkJ2SHFiVEhWS1pnZGZGZE5OcDYrb0dXUG9Bc2RDMXhZKzZcL1kxanlINTRERnd5UWszbXlzQ2M3Z1I3a0sifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [RFC] cryptodev: proposed changes in rte_cryptodev_sym_session X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Nov 2018 08:35:23 -0000 > > > > @@ -1301,8 +1346,7 @@ rte_cryptodev_sym_session_free(struct rte_cry= ptodev_sym_session *sess) > > > > > > > > /* Check that all device private data has been freed */ > > > > for (i =3D 0; i < nb_drivers; i++) { > > > [Fiona] Use the sess.nb_drivers rather than the global. > > > > Ok, though doesn't really matter here. > > get_sym_session_private_data() will return NULL for invalid index anywa= y. > [Fiona] Except that you removed the NULL check below and are using that > invalid index to deref the array. Ah yes, you right - have to use sess.nb_drivers here. >=20 > > > Actually maybe name slightly differently to reduce the chance of that= mistake happening, e.g. > > > rename sess.nb_drivers to sess.num_drivers? > > > > > > > - sess_priv =3D get_sym_session_private_data(sess, i); > > > > - if (sess_priv !=3D NULL) > > > > + if (sess->sess_data[i].refcnt !=3D 0) > > > > return -EBUSY; > > > > } > > > > > > > > @@ -1313,6 +1357,23 @@ rte_cryptodev_sym_session_free(struct rte_cr= yptodev_sym_session *sess) > > > > return 0; > > > > } > > > > > > > > +unsigned int > > > > +rte_cryptodev_sym_session_max_data_size(void) > > > [Fiona] Suggest renaming this > > > rte_cryptodev_sym_session_max_array_size() > > > > I usually don't care much about names, but that seems just confusing: > > totally unclear which array we are talking about. > > Any better naming ideas? > [Fiona] Isn't it returning the size of the array of structs in the sessio= n hdr? > Seems a bit more informative than "data". Yes, but user doesn't have to know it is an array of pointers or something different. > Can't think of anything better, if you find array confusing then I > don't feel that strongly about it, stick with data. > Or just get rid of the function altogether and put inside > rte_cryptodev_sym_session_max_size() > Unless it's called elsewhere. Sounds like a good option. Konstantin