On Thu, Nov 17, 2022 at 9:18 AM Suanming Mou <[email protected]> wrote: > > Hi, > > > -----Original Message----- > > From: David Marchand <[email protected]> > > Sent: Thursday, November 17, 2022 4:07 PM > > To: Suanming Mou <[email protected]> > > Cc: Ori Kam <[email protected]>; Aman Singh <[email protected]>; > > Yuying Zhang <[email protected]>; NBU-Contact-Thomas Monjalon > > (EXTERNAL) <[email protected]>; Ferruh Yigit <[email protected]>; > > Andrew Rybchenko <[email protected]>; Ray Kinsella > > <[email protected]>; [email protected] > > Subject: Re: [PATCH v1] ethdev: add indirect action async query > > > > Hello, > > > > On Tue, Sep 20, 2022 at 9:12 AM Suanming Mou <[email protected]> > > wrote: > > > @@ -2873,17 +2907,23 @@ port_queue_action_handle_destroy(portid_t > > port_id, > > > * of error. > > > */ > > > memset(&error, 0x99, sizeof(error)); > > > + job = calloc(1, sizeof(*job)); > > > + if (!job) { > > > + printf("Queue action destroy job allocate > > > failed\n"); > > > + return -ENOMEM; > > > + } > > > + job->type = QUEUE_JOB_TYPE_ACTION_DESTROY; > > > + job->pia = pia; > > > > > > if (pia->handle && > > > rte_flow_async_action_handle_destroy(port_id, > > > - queue_id, &attr, pia->handle, NULL, > > > &error)) { > > > + queue_id, &attr, pia->handle, job, > > > &error)) { > > > ret = port_flow_complain(&error); > > > continue; > > > } > > > *tmp = pia->next; > > > printf("Indirect action #%u destruction queued\n", > > > pia->id); > > > - free(pia); > > > break; > > > } > > > if (i == n) > > > > Our covscan tool reports a potential leak of "job" in this block. > > I am unclear whether it is a normal occurence, but it seems that if > > pia->handle == NULL, then job is leaked. > > OK, this function can only be called from destroying a created action handle. > For the created action handle, the pia->handle should never be NULL here. > And we also have " if (actions[i] != pia->id) " several lines above to ensure > it is a valid pia. > I agree from tools' point of view it looks like a leak here. But it should > never happen. > Do you think we need a "fix" in that case?
- If you are sure of it, unnecessary checks must be removed. - In pia->handle != NULL branch, won't "job" be leaked too if rte_flow_async_action_handle_destroy() fails? -- David Marchand
