On Fri, 15 Aug 2025 15:42:16 +0800
Bingbin Chen <chen.bing...@zte.com.cn> wrote:

> +static uint32_t
> +zxdh_np_agent_channel_acl_index_request(uint32_t dev_id, uint32_t sdt_no,
> +                     uint32_t vport, uint32_t *p_index)
> +{
> +     uint32_t rc = ZXDH_OK;
> +
> +     uint32_t rsp_buff[2] = {0};
> +     uint32_t msg_result = 0;
> +     uint32_t acl_index = 0;
> +     ZXDH_AGENT_CHANNEL_ACL_MSG_T msgcfg = {
> +             .dev_id = 0,
> +             .type = ZXDH_ACL_MSG,
> +             .oper = ZXDH_ACL_INDEX_REQUEST,
> +             .vport = vport,
> +             .sdt_no = sdt_no,
> +     };
> +     ZXDH_AGENT_CHANNEL_MSG_T agent_msg = {
> +             .msg = (void *)&msgcfg,
> +             .msg_len = sizeof(ZXDH_AGENT_CHANNEL_ACL_MSG_T),
> +     };
> +
> +     rc = zxdh_np_agent_channel_sync_send(dev_id, &agent_msg, rsp_buff, 
> sizeof(rsp_buff));

You were probably taught somewhere to initialize all variables. But that is 
actually
outdated recommendation. Modern compilers detect uninitialized variables and 
therefore bugs
rather than just getting what was set before.

For example here. The first declaration sets 'rc' but then the first call is 
then
setting rc, so the initialization in the declaration is redundent. I see lots 
of this in this code.

The best way in modern code is to put variable declaration as close
to where it is first used as possible.

        uint32_t rc = zxdh_np_agent_channel_sync_send(dev_id, &agent_msg, 
rsp_buff, sizeof(rsp_buff));

> +     dump_depth = dump_depth_128bit - start_index_128bit;
> +
> +     dump_data_buff = (uint8_t *)rte_zmalloc(NULL, dump_depth * 
> ZXDH_DTB_LEN_POS_SETP, 0);

Since rte_zmalloc() returns void * cast here is not needed.
This extra cast is done lots of places.

Also, can this be from heap (malloc) or stack (alloca) instead? 
Or does it need to be in hugepages for DMA reasons?
Same for lots of other buffers in the dump code.


Reply via email to