DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] common/qat: fix uninitialized variable bug
@ 2020-07-24  9:40 Adam Dybkowski
  2020-07-24 11:54 ` Trahe, Fiona
  0 siblings, 1 reply; 6+ messages in thread
From: Adam Dybkowski @ 2020-07-24  9:40 UTC (permalink / raw)
  To: dev, fiona.trahe, akhil.goyal; +Cc: Adam Dybkowski

This patch fixes the uninitialized variable bug in QAT PMD.

Fixes: 9f27a860dc16 ("crypto/qat: move generic qp function to qp file")

Signed-off-by: Adam Dybkowski <adamx.dybkowski@intel.com>
---
 drivers/common/qat/qat_qp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/common/qat/qat_qp.c b/drivers/common/qat/qat_qp.c
index aacd4ab21..0ee713955 100644
--- a/drivers/common/qat/qat_qp.c
+++ b/drivers/common/qat/qat_qp.c
@@ -582,7 +582,7 @@ qat_enqueue_op_burst(void *qp, void **ops, uint16_t nb_ops)
 	register struct qat_queue *queue;
 	struct qat_qp *tmp_qp = (struct qat_qp *)qp;
 	register uint32_t nb_ops_sent = 0;
-	register int ret;
+	register int ret = -1;
 	uint16_t nb_ops_possible = nb_ops;
 	register uint8_t *base_addr;
 	register uint32_t tail;
-- 
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] common/qat: fix uninitialized variable bug
  2020-07-24  9:40 [dpdk-dev] [PATCH] common/qat: fix uninitialized variable bug Adam Dybkowski
@ 2020-07-24 11:54 ` Trahe, Fiona
  2020-07-24 11:58   ` Dybkowski, AdamX
  0 siblings, 1 reply; 6+ messages in thread
From: Trahe, Fiona @ 2020-07-24 11:54 UTC (permalink / raw)
  To: Dybkowski, AdamX, dev, akhil.goyal; +Cc: Trahe, Fiona



> -----Original Message-----
> From: Dybkowski, AdamX <adamx.dybkowski@intel.com>
> Sent: Friday, July 24, 2020 10:40 AM
> To: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>; akhil.goyal@nxp.com
> Cc: Dybkowski, AdamX <adamx.dybkowski@intel.com>
> Subject: [PATCH] common/qat: fix uninitialized variable bug
> 
> This patch fixes the uninitialized variable bug in QAT PMD.
> 
> Fixes: 9f27a860dc16 ("crypto/qat: move generic qp function to qp file")
> 
> Signed-off-by: Adam Dybkowski <adamx.dybkowski@intel.com>
> ---
>  drivers/common/qat/qat_qp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/common/qat/qat_qp.c b/drivers/common/qat/qat_qp.c
> index aacd4ab21..0ee713955 100644
> --- a/drivers/common/qat/qat_qp.c
> +++ b/drivers/common/qat/qat_qp.c
> @@ -582,7 +582,7 @@ qat_enqueue_op_burst(void *qp, void **ops, uint16_t nb_ops)
>  	register struct qat_queue *queue;
>  	struct qat_qp *tmp_qp = (struct qat_qp *)qp;
>  	register uint32_t nb_ops_sent = 0;
> -	register int ret;
> +	register int ret = -1;
Nack - this fn returns an unsigned. So the correct option is to default to 0






^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] common/qat: fix uninitialized variable bug
  2020-07-24 11:54 ` Trahe, Fiona
@ 2020-07-24 11:58   ` Dybkowski, AdamX
  2020-07-24 12:20     ` Trahe, Fiona
  0 siblings, 1 reply; 6+ messages in thread
From: Dybkowski, AdamX @ 2020-07-24 11:58 UTC (permalink / raw)
  To: Trahe, Fiona, dev, akhil.goyal

> -----Original Message-----
> From: Trahe, Fiona <fiona.trahe@intel.com>
> Sent: Friday, 24 July, 2020 13:55
> To: Dybkowski, AdamX <adamx.dybkowski@intel.com>; dev@dpdk.org;
> akhil.goyal@nxp.com
> Cc: Trahe, Fiona <fiona.trahe@intel.com>
> Subject: RE: [PATCH] common/qat: fix uninitialized variable bug
> 
> 
> 
> > -----Original Message-----
> > From: Dybkowski, AdamX <adamx.dybkowski@intel.com>
> > Sent: Friday, July 24, 2020 10:40 AM
> > To: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>;
> > akhil.goyal@nxp.com
> > Cc: Dybkowski, AdamX <adamx.dybkowski@intel.com>
> > Subject: [PATCH] common/qat: fix uninitialized variable bug
> >
> > This patch fixes the uninitialized variable bug in QAT PMD.
> >
> > Fixes: 9f27a860dc16 ("crypto/qat: move generic qp function to qp
> > file")
> >
> > Signed-off-by: Adam Dybkowski <adamx.dybkowski@intel.com>
> > ---
> >  drivers/common/qat/qat_qp.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/common/qat/qat_qp.c b/drivers/common/qat/qat_qp.c
> > index aacd4ab21..0ee713955 100644
> > --- a/drivers/common/qat/qat_qp.c
> > +++ b/drivers/common/qat/qat_qp.c
> > @@ -582,7 +582,7 @@ qat_enqueue_op_burst(void *qp, void **ops,
> uint16_t nb_ops)
> >  	register struct qat_queue *queue;
> >  	struct qat_qp *tmp_qp = (struct qat_qp *)qp;
> >  	register uint32_t nb_ops_sent = 0;
> > -	register int ret;
> > +	register int ret = -1;
> Nack - this fn returns an unsigned. So the correct option is to default to 0

[Adam] The ret variable value (signed) is not returned directly, please check the rest of this function in src code. This is just checked to calculate how many ops were enqueued. And if all checks skip (meaning the op was not processed by sym crypto, asym crypto nor compression), we should note the user that the actual op was NOT enqueued. That's why ret is set to -1.

Adam

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] common/qat: fix uninitialized variable bug
  2020-07-24 11:58   ` Dybkowski, AdamX
@ 2020-07-24 12:20     ` Trahe, Fiona
  2020-07-26 19:19       ` Akhil Goyal
  0 siblings, 1 reply; 6+ messages in thread
From: Trahe, Fiona @ 2020-07-24 12:20 UTC (permalink / raw)
  To: Dybkowski, AdamX, dev, akhil.goyal



> -----Original Message-----
> From: Dybkowski, AdamX <adamx.dybkowski@intel.com>
> Sent: Friday, July 24, 2020 12:58 PM
> To: Trahe, Fiona <fiona.trahe@intel.com>; dev@dpdk.org; akhil.goyal@nxp.com
> Subject: RE: [PATCH] common/qat: fix uninitialized variable bug
> 
> > -----Original Message-----
> > From: Trahe, Fiona <fiona.trahe@intel.com>
> > Sent: Friday, 24 July, 2020 13:55
> > To: Dybkowski, AdamX <adamx.dybkowski@intel.com>; dev@dpdk.org;
> > akhil.goyal@nxp.com
> > Cc: Trahe, Fiona <fiona.trahe@intel.com>
> > Subject: RE: [PATCH] common/qat: fix uninitialized variable bug
> >
> >
> >
> > > -----Original Message-----
> > > From: Dybkowski, AdamX <adamx.dybkowski@intel.com>
> > > Sent: Friday, July 24, 2020 10:40 AM
> > > To: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>;
> > > akhil.goyal@nxp.com
> > > Cc: Dybkowski, AdamX <adamx.dybkowski@intel.com>
> > > Subject: [PATCH] common/qat: fix uninitialized variable bug
> > >
> > > This patch fixes the uninitialized variable bug in QAT PMD.
> > >
> > > Fixes: 9f27a860dc16 ("crypto/qat: move generic qp function to qp
> > > file")
> > >
> > > Signed-off-by: Adam Dybkowski <adamx.dybkowski@intel.com>
> > > ---
> > >  drivers/common/qat/qat_qp.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/common/qat/qat_qp.c b/drivers/common/qat/qat_qp.c
> > > index aacd4ab21..0ee713955 100644
> > > --- a/drivers/common/qat/qat_qp.c
> > > +++ b/drivers/common/qat/qat_qp.c
> > > @@ -582,7 +582,7 @@ qat_enqueue_op_burst(void *qp, void **ops,
> > uint16_t nb_ops)
> > >  	register struct qat_queue *queue;
> > >  	struct qat_qp *tmp_qp = (struct qat_qp *)qp;
> > >  	register uint32_t nb_ops_sent = 0;
> > > -	register int ret;
> > > +	register int ret = -1;
> > Nack - this fn returns an unsigned. So the correct option is to default to 0
> 
> [Adam] The ret variable value (signed) is not returned directly, please check the rest of this function in src
> code. This is just checked to calculate how many ops were enqueued. And if all checks skip (meaning the
> op was not processed by sym crypto, asym crypto nor compression), we should note the user that the
> actual op was NOT enqueued. That's why ret is set to -1.
[Fiona] ok. makes sense thanks. In that case
Acked-by: Fiona Trahe <fiona.trahe@intel.com>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] common/qat: fix uninitialized variable bug
  2020-07-24 12:20     ` Trahe, Fiona
@ 2020-07-26 19:19       ` Akhil Goyal
  2020-07-29 13:19         ` Thomas Monjalon
  0 siblings, 1 reply; 6+ messages in thread
From: Akhil Goyal @ 2020-07-26 19:19 UTC (permalink / raw)
  To: Trahe, Fiona, Dybkowski, AdamX, dev; +Cc: stable

> >
> > [Adam] The ret variable value (signed) is not returned directly, please check the
> rest of this function in src
> > code. This is just checked to calculate how many ops were enqueued. And if all
> checks skip (meaning the
> > op was not processed by sym crypto, asym crypto nor compression), we
> should note the user that the
> > actual op was NOT enqueued. That's why ret is set to -1.
> [Fiona] ok. makes sense thanks. In that case
> Acked-by: Fiona Trahe <fiona.trahe@intel.com>

Cc: stable@dpdk.org

Applied to dpdk-next-crypto

Thanks

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] common/qat: fix uninitialized variable bug
  2020-07-26 19:19       ` Akhil Goyal
@ 2020-07-29 13:19         ` Thomas Monjalon
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Monjalon @ 2020-07-29 13:19 UTC (permalink / raw)
  To: Trahe, Fiona, Dybkowski, AdamX, Akhil Goyal; +Cc: dev, stable

26/07/2020 21:19, Akhil Goyal:
> > >
> > > [Adam] The ret variable value (signed) is not returned directly, please check the
> > rest of this function in src
> > > code. This is just checked to calculate how many ops were enqueued. And if all
> > checks skip (meaning the
> > > op was not processed by sym crypto, asym crypto nor compression), we
> > should note the user that the
> > > actual op was NOT enqueued. That's why ret is set to -1.
> > [Fiona] ok. makes sense thanks. In that case
> > Acked-by: Fiona Trahe <fiona.trahe@intel.com>
> 
> Cc: stable@dpdk.org
> 
> Applied to dpdk-next-crypto


This patch should not have been merged.
The explanation is missing.

This is the commit log:
"This patch fixes the uninitialized variable bug in QAT PMD."

We don't even know what is the consequence and the scope.
That's not acceptable for a fix.



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-07-29 13:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-24  9:40 [dpdk-dev] [PATCH] common/qat: fix uninitialized variable bug Adam Dybkowski
2020-07-24 11:54 ` Trahe, Fiona
2020-07-24 11:58   ` Dybkowski, AdamX
2020-07-24 12:20     ` Trahe, Fiona
2020-07-26 19:19       ` Akhil Goyal
2020-07-29 13:19         ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).