Hello,
"Ananyev, Konstantin" <konstantin.anan...@intel.com> writes:
> Hi David,
>
>> > Have a question regarding validate-abi.sh.
>> > It complains on the following changes with that patch:
>> >
>> > @@ -111,11 +129,21 @@ struct rte_ring {
>> > char pad0 __rte_cache_aligned; /**< empty cache line */
>> >
>> > /** Ring producer status. */
>> > - struct rte_ring_headtail prod __rte_cache_aligned;
>> > + RTE_STD_C11
>> > + union {
>> > + struct rte_ring_headtail prod;
>> > + struct rte_ring_rts_headtail rts_prod;
>> > + } __rte_cache_aligned;
>> > +
>> > char pad1 __rte_cache_aligned; /**< empty cache line */
>> >
>> > /** Ring consumer status. */
>> > - struct rte_ring_headtail cons __rte_cache_aligned;
>> > + RTE_STD_C11
>> > + union {
>> > + struct rte_ring_headtail cons;
>> > + struct rte_ring_rts_headtail rts_cons;
>> > + } __rte_cache_aligned;
>> > +
[...]
>> It reported a warning on those fields:
>> https://travis-ci.com/github/ovsrobot/dpdk/jobs/310689008#L2380
>> I understand this as a false positive too.
>>
>> It seems similar to the bz I opened about fields moved to anonymous
>> constructs: https://sourceware.org/bugzilla/show_bug.cgi?id=25661
>
> Yes, looks the same.
>
>
>> Cc: Dodji.
>>
>>
>> For the time being, you can waive this by adding a rule in
>> devtools/libabigail.abignore.
>
> Ok, so what is the procedure here?
> Should I submit changes to devtools/libabigail.abignore together with the
> patch that introduced the problem?
> BTW, I used the following changes in libabigail.abignore to supress errors:
> $ git diff devtools/libabigail.abignore
> diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore
> index a59df8f13..032479b9f 100644
> --- a/devtools/libabigail.abignore
> +++ b/devtools/libabigail.abignore
> @@ -11,3 +11,9 @@
> type_kind = enum
> name = rte_crypto_asym_xform_type
> changed_enumerators = RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END
> +; Ignore updates of ring prod/cons
> +[suppress_type]
> + type_kind = struct
> + name = rte_ring
> + has_data_members_inserted_at = offsetof(prod)
> + has_data_members_inserted_at = offsetof(cons)
>
> Do you know, is there a better (more fine-grained) approach?
I think what you did is correct.
I am teaching the tool to recognize this kind of change construct and
avoid emitting a false positive. So that you can drop these kind of
suppression specification.
Sorry for the inconvenience.
Cheers,
--
Dodji
