From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0075AA0527; Mon, 9 Nov 2020 10:28:38 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 13A6C592C; Mon, 9 Nov 2020 10:28:36 +0100 (CET) Received: from szxga07-in.huawei.com (szxga07-in.huawei.com [45.249.212.35]) by dpdk.org (Postfix) with ESMTP id CB3C35913 for ; Mon, 9 Nov 2020 10:28:34 +0100 (CET) Received: from DGGEMS402-HUB.china.huawei.com (unknown [172.30.72.59]) by szxga07-in.huawei.com (SkyGuard) with ESMTP id 4CV5Jn4z2mz74Tc for ; Mon, 9 Nov 2020 17:28:25 +0800 (CST) Received: from [10.67.103.119] (10.67.103.119) by DGGEMS402-HUB.china.huawei.com (10.3.19.202) with Microsoft SMTP Server id 14.3.487.0; Mon, 9 Nov 2020 17:28:23 +0800 To: Ferruh Yigit CC: , References: <1604586194-29523-1-git-send-email-oulijun@huawei.com> <1604634716-43484-1-git-send-email-oulijun@huawei.com> <1604634716-43484-3-git-send-email-oulijun@huawei.com> <59c4a8e3-fddc-cae7-3b8e-f52631407e4b@intel.com> From: oulijun Message-ID: <7db02e43-329f-3c05-e1b8-902fd484a487@huawei.com> Date: Mon, 9 Nov 2020 17:28:24 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 MIME-Version: 1.0 In-Reply-To: <59c4a8e3-fddc-cae7-3b8e-f52631407e4b@intel.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.119] X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH v2 2/5] net/hns3: use unsigned types for bit operator X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 在 2020/11/7 0:38, Ferruh Yigit 写道: > On 11/6/2020 3:51 AM, Lijun Ou wrote: >> From: Hongbo Zheng >> >> According to bit operator reliability style, variables in >> the right expression participating int bit operation >> cannot be of unsigned type. > > Assuming this is talking about BIT() ("#define BIT(nr) (1UL << (nr))"), > is this description says, in the "a << b", 'b' can't be unsigned? > Sorry, The commit write error. according to the coverity check, it need to satisfies for the right expression to be unsigned. > The code below does the opposite, "int i" -> "uint32_t i", even though > there is a typo in above description, why 'b' can't be signed? > It can't be negative, but not sure if is it a problem to have it signed. > > > Also only first change in this patch seems related to the patch title > and the description, rest looks related to signed / unsigned comparison > fixes, if so can you separate them into their patch with proper > description please? > Yes, I put them together because they are all coveredity alarms and bit-operator alarms. >> >> Signed-off-by: Hongbo Zheng >> Signed-off-by: Lijun Ou >> --- >> drivers/net/hns3/hns3_ethdev_vf.c | 2 +- >> drivers/net/hns3/hns3_rxtx_vec_neon.h | 11 +++++------ >> 2 files changed, 6 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/net/hns3/hns3_ethdev_vf.c >> b/drivers/net/hns3/hns3_ethdev_vf.c >> index 6f71cd6..2e9bfda 100644 >> --- a/drivers/net/hns3/hns3_ethdev_vf.c >> +++ b/drivers/net/hns3/hns3_ethdev_vf.c >> @@ -1331,7 +1331,7 @@ hns3vf_get_tc_info(struct hns3_hw *hw) >> { >> uint8_t resp_msg; >> int ret; >> - int i; >> + uint32_t i; >> ret = hns3_send_mbx_msg(hw, HNS3_MBX_GET_TCINFO, 0, NULL, 0, >> true, &resp_msg, sizeof(resp_msg)); >> diff --git a/drivers/net/hns3/hns3_rxtx_vec_neon.h >> b/drivers/net/hns3/hns3_rxtx_vec_neon.h >> index 8d7721b..fe525de 100644 >> --- a/drivers/net/hns3/hns3_rxtx_vec_neon.h >> +++ b/drivers/net/hns3/hns3_rxtx_vec_neon.h >> @@ -89,13 +89,12 @@ hns3_desc_parse_field(struct hns3_rx_queue *rxq, >> struct hns3_desc *rxdp, >> uint32_t bd_vld_num) >> { >> - uint32_t l234_info, ol_info, bd_base_info; >> + uint32_t l234_info, ol_info, bd_base_info, cksum_err, i; > > Not sure combining more variable declarations into same line is good > idea, why not have their own lines? > Yes. I agree with you. I will fixes it in next version. >> struct rte_mbuf *pkt; >> uint32_t retcode = 0; >> - uint32_t cksum_err; >> - int ret, i; >> + int ret; >> - for (i = 0; i < (int)bd_vld_num; i++) { >> + for (i = 0; i < bd_vld_num; i++) { >> pkt = sw_ring[i].mbuf; >> /* init rte_mbuf.rearm_data last 64-bit */ >> @@ -129,9 +128,9 @@ hns3_recv_burst_vec(struct hns3_rx_queue >> *__restrict rxq, >> uint16_t rx_id = rxq->next_to_use; >> struct hns3_entry *sw_ring = &rxq->sw_ring[rx_id]; >> struct hns3_desc *rxdp = &rxq->rx_ring[rx_id]; >> - uint32_t bd_valid_num, parse_retcode; >> + uint32_t bd_valid_num, parse_retcode, pos; >> uint16_t nb_rx = 0; >> - int pos, offset; >> + int offset; >> /* mask to shuffle from desc to mbuf's rx_descriptor_fields1 */ >> uint8x16_t shuf_desc_fields_msk = { >> > > . >