On 2021/6/4 23:04, Pattan, Reshma wrote:
>
>
>> -----Original Message-----
>> From: Min Hu (Connor) <humi...@huawei.com>
>
> <snip>
>
>> + ret = rte_eth_dev_get_reg_info(i, ®_info);
>> + if (ret) {
>> + printf("Error getting device reg info: %d\n", ret);
>> + continue;
>> + }
>> +
>> + buf_size = reg_info.length * reg_info.width;
>
>
> If it is to get the regs length, you can directly call
> "rte_ethtool_get_regs_len(uint16_t port_id)" API , instead of again writing
> the above logic.
> And use the returned length in below malloc.
This logic is indeed identical to the logic of the "rte_ethtool_get_regs_len"
API of Ethtool,
but the method of using the "rte_eth_dev_get_reg_info" API is the case. All
users will have
similar code logic when using this API.
And use "rte_ethtool_get_regs_len" API here only reduces "buf_size =
reg_info.length * reg_info.width;"
this logic. But at the same time, it introduces the dependence of
example/ethtool in procinfo. The code in
example is not compiled by default, which seems not appropriate to import it in
app/procinfo.
So, I think it is fine to keep this.
>
>
>> + fp_regs = fopen(file_name, "wb");
>> + if (fp_regs == NULL) {
>> + printf("Error during opening '%s' for writing\n",
>> + file_name);
>
> Better to print error string from fopen() errno on failure , to indicate the
> exact error.
Agree， I will fix it in the next version.
>
>> + } else {
>> + if ((int)fwrite(buf_data, 1, buf_size, fp_regs) !=
>
> Better have "((int)fwrite(buf_data, 1, buf_size, fp_regs)" In separate line
> and use the returned value inside if check.
Agree， I will fix it in the next version.
>
>> + buf_size)
>> + printf("Error during writing %s\n",
>> + file_prefix);
>
> Better to print error string from fwrite errno on failure , to indicate the
> exact error.
>
>> + else
>> + printf("dump device (%s) regs successfully, "
>
> Reframe the sente to "Device regs dumped successfully"
>
Agree， I will fix it in the next version.
> .
>
