Hi Anatoly,
Thanks for your review, a lot of valuable advice.
PS: This library stays for a long time, want to hear TB's opinion: whether to
continue or stop.
If continue I will push a new version.
Thanks.
On 2023/6/22 23:55, Burakov, Anatoly wrote:
> On 2/9/2023 6:36 AM, Chengwen Feng wrote:
>> This patch supports rte_memarea_dump() API which could be used for
>> debug.
>>
>> Signed-off-by: Chengwen Feng <[email protected]>
>> Reviewed-by: Dongdong Liu <[email protected]>
>> Acked-by: Morten Brørup <[email protected]>
>> ---
>
> Provisionally,
>
> Acked-by: Anatoly Burakov <[email protected]>
>
> As long as below is addressed.
>
>> +static void
>> +memarea_dump_objects_detail(struct rte_memarea *ma, FILE *f)
>> +{
>> + struct memarea_objhdr *hdr;
>> + size_t offset;
>> + void *ptr;
>> +
>> + fprintf(f, " objects:\n");
>> + TAILQ_FOREACH(hdr, &ma->obj_list, obj_next) {
>> + if (hdr == ma->guard_hdr)
>> + break;
>> + memarea_check_cookie(ma, hdr, 2);
>> + ptr = RTE_PTR_ADD(hdr, sizeof(struct memarea_objhdr));
>> + offset = RTE_PTR_DIFF(ptr, ma->area_base);
>> +#ifdef RTE_LIBRTE_MEMAREA_DEBUG
>> + fprintf(f, " %p off: 0x%zx size: 0x%zx %s\n",
>> + ptr, offset, MEMAREA_OBJECT_GET_SIZE(hdr),
>> + MEMAREA_OBJECT_IS_ALLOCATED(hdr) ? "allocated" : "");
>> +#else
>> + fprintf(f, " off: 0x%zx size: 0x%zx %s\n",
>> + offset, MEMAREA_OBJECT_GET_SIZE(hdr),
>> + MEMAREA_OBJECT_IS_ALLOCATED(hdr) ? "allocated" : "");
>> +#endif
>> + }.
>> +}
>> +
>> +int
>> +rte_memarea_dump(struct rte_memarea *ma, FILE *f, bool dump_all)
>> +{
>> + if (ma == NULL || f == NULL)
>> + return -EINVAL;
>
> I feel like the API is inconsistent in this way. I would suggest picking a
> method of error reporting, and sticking with it. I would suggest returning
> 0/-1 or ptr/NULL with rte_errno set to indicate error, as that is how most
> libraries in DPDK behave.
>
