Re: [dpdk-dev] [PATCH v2 1/2] crypto/dpaa2_sec: improve error handling

[Akhil Goyal <akhil.goyal@nxp.com>]

Hi Lukasz,
> 
> Hi Akhil,
> 
> There are still few places, where returned error code is not set.
> Maybe you can add those places to your patch?

Thanks for the review. I will surely look into it.


> 
> Here's the list:
> 
> --- a/drivers/crypto/dpaa_sec/dpaa_sec.c
> +++ b/drivers/crypto/dpaa_sec/dpaa_sec.c
> @@ -510,6 +510,7 @@ dpaa_sec_prep_cdb(dpaa_sec_session *ses)
>                          break;
>                  default:
>                          DPAA_SEC_ERR("unsupported auth alg %u",
> ses->auth_alg);
> +                       /* this path will return 0 */
>                  }
>                  break;
>          case DPAA_SEC_AEAD:
> @@ -2308,7 +2309,7 @@ dpaa_sec_attach_sess_q(struct dpaa_sec_qp *qp,
> dpaa_sec_session *sess)
>          ret = dpaa_sec_prep_cdb(sess);
>          if (ret) {
>                  DPAA_SEC_ERR("Unable to prepare sec cdb");
> -               return -1;
> +               return -1; /* Why not return ret ? */
>          }
>          if (unlikely(!RTE_PER_LCORE(dpaa_io))) {
>                  ret = rte_dpaa_portal_init((void *)0);
> @@ -2826,7 +2827,7 @@ dpaa_sec_set_ipsec_session(__rte_unused struct
> rte_cryptodev *dev,
>                          }
>                  }
>          } else
> -               goto out;
> +               goto out; /* set ret before goto */
>          rte_spinlock_lock(&internals->lock);
>          for (i = 0; i < MAX_DPAA_CORES; i++) {
>                  session->inq[i] = dpaa_sec_attach_rxq(internals);
> @@ -2843,7 +2844,7 @@ dpaa_sec_set_ipsec_session(__rte_unused struct
> rte_cryptodev *dev,
>          rte_free(session->auth_key.data);
>          rte_free(session->cipher_key.data);
>          memset(session, 0, sizeof(dpaa_sec_session));
> -       return -1;
> +       return -1; /* return ret ? */
>   }
> 
>   static int
> @@ -2992,7 +2993,7 @@ dpaa_sec_set_pdcp_session(struct rte_cryptodev
> *dev,
>          rte_free(session->auth_key.data);
>          rte_free(session->cipher_key.data);
>          memset(session, 0, sizeof(dpaa_sec_session));
> -       return -1;
> +       return -1; /* still returning generic -1 */
>   }
> 
>   static int
> 
> --
> 
> Lukasz Wojciechowski
> Principal Software Engineer
> 
> Samsung R&D Institute Poland
> Samsung Electronics
> Office +48 22 377 88 25
> l.wojciechow@partner.samsung.com