Re: ABI incompatible change or not?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxx
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@xxxxxxxxxxxxxxxxxxxxxxx
Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
_______________________________________________
devel mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxx
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@xxxxxxxxxxxxxxxxxxxxxxx
Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Fedora Announce]     [Fedora Users]     [Fedora Kernel]     [Fedora Testing]     [Fedora Formulas]     [Fedora PHP Devel]     [Kernel Development]     [Fedora Legacy]     [Fedora Maintainers]     [Fedora Desktop]     [PAM]     [Red Hat Development]     [Gimp]     [Yosemite News]

  Powered by Linux