prov/sockets: Coverity scan fix and minor code cleanup #1104
Conversation
shantonu
force-pushed
the
pr/sockets-coverityfix
branch
from
2b13a20
to
ea7627e
Compare
|
This probably should have been broken into 2 separate patches. See inline note |
| @@ -1429,7 +1429,6 @@ static int sock_pe_process_rx_send(struct sock_pe *pe, struct sock_rx_ctx *rx_ct | |||
| rx_entry->data = pe_entry->data; | |||
| rx_entry->ignore = 0; | |||
| rx_entry->comp = pe_entry->comp; | |||
| pe_entry->context = rx_entry->context; | |||
There was a problem hiding this comment.
Why was this changed?
The change to setting data_len looks like a valid fix, versus a code cleanup. The change to context needs more explanation.
Never mind - I was reading the data_len change wrong -- it looks like a simple cleanup. Reading the source, I see why the context change was done, but the patch description should explain that better. Reading just the patch, the change looks wrong.
There was a problem hiding this comment.
Sorry, the description should have been clearer. Updating the description
Signed-off-by: Shantonu Hossain <shantonu.hossain@intel.com>
Signed-off-by: Shantonu Hossain <shantonu.hossain@intel.com>
shantonu
force-pushed
the
pr/sockets-coverityfix
branch
from
ea7627e
to
97c91f1
Compare
|
Divided into two patches and updated the description. |
| if(!s) { | ||
| SOCK_LOG_DBG("Invalid FI_SOCKETS_PE_AFFINITY value\n"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
This check shouldn't be needed. This comes from fi_var_get_str(). If that call returns success, the return string will not be NULL.
There was a problem hiding this comment.
The coverity issue deals with the saveptra variable:
CID 100409 (#1 of 1): Explicit null dereferenced (FORWARD_NULL)6. var_deref_model: Passing &saveptra to __strtok_r_1c, which dereferences null saveptra. [show details]
This is not an actual issue. The checker just can't distinguish the first call to strtok_r, versus a subsequent call. I marked this defect as invalid in coverity. Hopefully that works to dismiss it.
There was a problem hiding this comment.
I also found that the check is not needed but since it was reported by coverity, I added it. Thanks for reporting it in coverity.
There was a problem hiding this comment.
If you re-read the coverity report, the issue is not the 's' parameter into strtok_r, but the 'saveptra' parameter. This change would not have fixed the issue.
|
Merging manually |
prov/gni: Add NIC list lock to mem reg path
@jithinjosepkl can you please review this?
Signed-off-by: Shantonu Hossain shantonu.hossain@intel.com