Petr said pretty much the same thing I was about to say.
As others have noted, the “indirect sub-type change” seems to be an
awkward way of saying the function prototype has changed, which is
certainly an ABI change for the mbest_search function. The only question
is, is mbest_search really part of the library’s ABI?
Since the function in question does not appear in any public API header,
it should be considered an internal, non-API function. Symbol visibility
control can limit the exposed symbols to the intended API/ABI, but this
upstream doesn’t seem to attempt that. The fedabipkgdiff tool is, of
course, comparing all visible symbols, and has no idea about headers or
documentation.
Since there is no “public” declaration of mbest_search, dependent
packages *should not* be linking it. If they take the dangerous and
brittle step of declaring the prototype manually, they *could*. However,
in this unlikely case, a rebuild wouldn’t help, because the dependent
package would need a patch to update its declaration (or, better, to
stop linking a function that is supposed to be internal).
I also think that these facts comprise an argument that this should
*not* be considered an ABI change in practice.
– Ben
On 3/3/22 09:12, Petr Pisar wrote:
> V Thu, Mar 03, 2022 at 07:44:45AM -0600, Richard Shaw napsal(a):
>> In this instance, it's not clear to me whether sub-type changes are ABI
>> breaking or not...
>>
>> $ fedabipkgdiff --from fc37 codec2-1.0.3-1.fc37.x86_64.rpm
>> Comparing the ABI of binaries between codec2-1.0.1-2.fc36.x86_64.rpm and
>> codec2-1.0.3-1.fc37.x86_64.rpm:
>>
>> ================ changes of 'libcodec2.so.1.0'===============
>> Functions changes summary: 0 Removed, 1 Changed, 5 Added functions
>> Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>>
>> 5 Added functions:
>>
>> [A] 'function void fdmdv_48_to_8(float*, float*, int)'
>> {fdmdv_48_to_8}
>> [A] 'function void fdmdv_48_to_8_short(short int*, short int*,
int)'
>> {fdmdv_48_to_8_short}
>> [A] 'function void fdmdv_8_to_48(float*, float*, int)'
>> {fdmdv_8_to_48}
>> [A] 'function void fdmdv_8_to_48_short(short int*, short int*,
int)'
>> {fdmdv_8_to_48_short}
>> [A] 'function void mbest_precompute_weight(float*, float*, int,
int)'
>> {mbest_precompute_weight}
>>
>> 1 function with some indirect sub-type change:
>>
>> [C] 'function void mbest_search(const float*, float*, float*, int, int,
>> MBEST*, int*)' at mbest.c:123:1 has some indirect sub-type changes:
>> parameter 3 of type 'float*' changed:
>> entity changed from 'float*' to 'int'
>> type size changed from 64 to 32 (in bits)
>> parameter 5 of type 'int' changed:
>> entity changed from 'int' to 'MBEST*'
>> type size changed from 32 to 64 (in bits)
>> parameter 6 of type 'MBEST*' changed:
>> in pointed to type 'struct MBEST':
>> entity changed from 'struct MBEST' to 'int'
>> type size changed from 128 to 32 (in bits)
>> parameter 7 of type 'int*' was removed
>>
>> ================ end of changes of 'libcodec2.so.1.0'===============
>>
>> Do I need to rebuild deps or not?
>>
> I have no idea what "indirect sub-type" means. Maybe it means that the
> function is called indirectly by other functions.
>
> If you look at the header files, there is no mbest_search() declaration. Hence
> mbest_search() cannot be part of public API. Therefore I believe you don't
> need to rebuild other packages because none of them should call it.
>
> Instead you could recommend upstream to hide that symbol from a symbol table
> using an linker script or a GCC-specific function attribute
> visibility("hidden"). That way nobody will be able to call it and various
> static scanners like the abidiff won't trip over it. As a reasult it would
> disappear from this output:
>
> $ nm -D /usr/lib64/libcodec2.so |grep mbest_search
> 0000000000024790 T mbest_search
> 0000000000024860 T mbest_search450
>
> -- Petr
>
> _______________________________________________
> devel mailing list -- devel(a)lists.fedoraproject.org
> To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
> Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives:
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org
> Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure