Hi Daniel, >>> --- a/lib/librte_cmdline/cmdline_rdline.c >>> +++ b/lib/librte_cmdline/cmdline_rdline.c >>> @@ -377,7 +377,10 @@ rdline_char_in(struct rdline *rdl, char c) >>> case CMDLINE_KEY_CTRL_K: >>> cirbuf_get_buf_head(&rdl->right, rdl->kill_buf, >> RDLINE_BUF_SIZE); >>> rdl->kill_size = CIRBUF_GET_LEN(&rdl->right); >>> - cirbuf_del_buf_head(&rdl->right, rdl->kill_size); >>> + >>> + if (cirbuf_del_buf_head(&rdl->right, rdl->kill_size) < >>> 0) >>> + return -EINVAL; >>> + >>> rdline_puts(rdl, vt100_clear_right); >>> break; >>> >> >> I wonder if a better way to fix wouldn't be to remove the checks >> introduced in http://dpdk.org/browse/dpdk/commit/?id=ab971e562860 >> >> There is no reason to check that in cirbuf_get_buf_head/tail(): >> if (!cbuf || !c) >> >> The function should never fail, it just returns the number of >> copied chars. This is the responsibility of the caller to ensure >> that the pointer to the circular buffer is not NULL. >> >> Also, rdline_char_in() is not expected to return -EINVAL, but >> RDLINE_RES_* instead. >> >> So I think that partially revert ab971e562860 would fix the >> coverity warning. >> >> Regards, >> Olivier > > Removing checks probably will generate more Coverity errors somewhere. > I see that only places where we test negative values are in unit tests. > > Reverting changes I think is overhead and maybe ignoring this patch and set > is as false positive in Coverity is better idea ?
We can mark the warning as false positive because this cannot happen right now (the calller checks the validity of cbuf/c). But this is probably something I'll come back on with a patch since there is no reason to check that pointers are not NULL in cirbuf_get_buf_head/tail(). Regards, Olivier
