Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Emil Velikov]

On 21 March 2018 at 09:28, Daniel Stone  wrote:
> Hi,
>
> On 21 March 2018 at 08:27, Pekka Paalanen  wrote:
>> On Tue, 20 Mar 2018 11:46:32 +, Daniel Stone  
>> wrote:
>>> inconvenience of maintaining a list of every linker's implementation
>>> detail on every platform, outweighs the risk of an exported
>>> underscore-prefixed symbol slipping through review.
>>
>> how about we make it a coding style rule to not have any symbols
>> starting with an underscore?
>>
>> I don't recall any already being in the code for either Wayland or
>> Weston, but I didn't check. They certainly are not public ABI as the
>> ABI check script would have listed such.
>
> Sure, works for me.
>
Indeed very good suggestion.

>> Aren't symbols (functions, variables) starting with an underscore
>> reserved in C so should not be used anyway?
>
> My recollection is that double-underscore and underscore-uppercase is
> forbidden for use apart from the implementation / standard library,
> and anything else underscore (underscore-number or
> underscore-lowercase) can only be used for static symbols. But I'm
> sure Simon will be along shortly to correct the record. :)
>
The spec does not explicitly state "static symbols" although you're
spot on overall.

-Emil

https://stackoverflow.com/a/10688187
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Daniel Stone]

Hi,

On 21 March 2018 at 08:27, Pekka Paalanen  wrote:
> On Tue, 20 Mar 2018 11:46:32 +, Daniel Stone  wrote:
>> inconvenience of maintaining a list of every linker's implementation
>> detail on every platform, outweighs the risk of an exported
>> underscore-prefixed symbol slipping through review.
>
> how about we make it a coding style rule to not have any symbols
> starting with an underscore?
>
> I don't recall any already being in the code for either Wayland or
> Weston, but I didn't check. They certainly are not public ABI as the
> ABI check script would have listed such.

Sure, works for me.

> Aren't symbols (functions, variables) starting with an underscore
> reserved in C so should not be used anyway?

My recollection is that double-underscore and underscore-uppercase is
forbidden for use apart from the implementation / standard library,
and anything else underscore (underscore-number or
underscore-lowercase) can only be used for static symbols. But I'm
sure Simon will be along shortly to correct the record. :)

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Pekka Paalanen]

On Tue, 20 Mar 2018 11:46:32 +
Daniel Stone  wrote:

> inconvenience of maintaining a list of every linker's implementation
> detail on every platform, outweighs the risk of an exported
> underscore-prefixed symbol slipping through review.

Hi,

how about we make it a coding style rule to not have any symbols
starting with an underscore?

I don't recall any already being in the code for either Wayland or
Weston, but I didn't check. They certainly are not public ABI as the
ABI check script would have listed such.

Aren't symbols (functions, variables) starting with an underscore
reserved in C so should not be used anyway?


Thanks,
pq


pgpTSykUUPsNN.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Emil Velikov]

On 20 March 2018 at 15:09, Derek Foreman  wrote:
> On 2018-03-20 10:02 AM, Emil Velikov wrote:
>> On 20 March 2018 at 14:50, Derek Foreman  wrote:
>>> On 2018-03-20 07:11 AM, Daniel Stone wrote:
 On 20 March 2018 at 11:55, Emil Velikov  wrote:
> On 20 March 2018 at 11:46, Daniel Stone  wrote:
>> Sure. As on IRC though, we definitely need to add at least _ftext for
>> MIPS anyway:
>> https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74
>>
>> And probably some more for ARM toolchains using other linkers:
>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html
>>
>> Doing a quick check across all the architectures on Debian shows that
>> your updated list is also missing _fbss, _fdata, and _ftext. So we'd
>> need to respin for those as well, but I think at this point the
>> inconvenience of maintaining a list of every linker's implementation
>> detail on every platform, outweighs the risk of an exported
>> underscore-prefixed symbol slipping through review.
>
> I think you're preemptively worried about this. Plus there's no need
> to add every symbol a search could find.
>
> Even then, ask yourself the question:
> Which is better - updating an ugly looking list once in a blue moon or
> having apps use internal API?
>
> As said above: _if_ it turns out to be labour intensive - fine remove it.
> We've already spend more time researching than what the [expected]
> maintenance for 1 year would be ;-)

 It all depends on how you view scope and probability.

 I don't agree my concern is preemptive: we've just shipped a beta
 where 'make check' fails on a few architectures, and the proposed fix
 (adding several symbols to the platform list) will still fail on
 architectures Debian ships on, until it also adds _fbss, _fdata and
 _ftext. I think my suggestion is reasonable: it fixes the very real
 problem, in a way which avoids expanding the scope of our ABI checker
 to include the implementation details of every linker/architecture
 pair that people use Wayland on. The cost of this is the probability
 that someone manages to add a new underscored symbol marked with
 WL_EXPORT into a library which is currently 105 LoC, and have no-one
 notice that in review. I think that's a reasonable tradeoff.
>>>
>>> I'm inclined to agree with Daniel here.
>>>
>>> Someone managing to sneak a WL_EXPORT on a symbol starting with a _ past
>>> review seems reasonably unlikely.
>>>
>>> And after reading this thread I'm still not entirely sure we have a
>>> complete list of what should be on that whitelist, and there are only a
>>> limited number of systems I can test build on right now.
>>>
>> Sharing a few ideas that are less obvious. Having the partial list
>> allows you to:
>>  - see active developers/users - be that by merging their patches or
>> skimming through the report
>
> I don't imagine we'll be adding new API to that library frequently
> enough to mine that data productively.
>
I am talking about the C runtime symbols.

>>  - have contact point for people with unusual platforms/setups
>
> By defaulting to a broken build state for unusual platforms, do you
> really think we'll attract many contact points?
>
Quite a serious typo here - s/build/check/. From experience - yes, you will.

>>  - is fairly trivial newbie task ;-)
>
> I don't think we should be going out of our way to generate problems for
> newbies to solve.  There are plenty of worthwhile tasks for new
> participants already.
>
Might want to make the list more obvious. I've been [sort of] around
and haven't seen any.

> I've confirmed that we once again pass make check on arm (something I
> really should've done prior to the beta release.  sigh.) and have pushed
> this patch.
>
You can never have 'enough' tests before a release ;-)

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Derek Foreman]

On 2018-03-20 10:02 AM, Emil Velikov wrote:
> On 20 March 2018 at 14:50, Derek Foreman  wrote:
>> On 2018-03-20 07:11 AM, Daniel Stone wrote:
>>> On 20 March 2018 at 11:55, Emil Velikov  wrote:
 On 20 March 2018 at 11:46, Daniel Stone  wrote:
> Sure. As on IRC though, we definitely need to add at least _ftext for
> MIPS anyway:
> https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74
>
> And probably some more for ARM toolchains using other linkers:
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html
>
> Doing a quick check across all the architectures on Debian shows that
> your updated list is also missing _fbss, _fdata, and _ftext. So we'd
> need to respin for those as well, but I think at this point the
> inconvenience of maintaining a list of every linker's implementation
> detail on every platform, outweighs the risk of an exported
> underscore-prefixed symbol slipping through review.

 I think you're preemptively worried about this. Plus there's no need
 to add every symbol a search could find.

 Even then, ask yourself the question:
 Which is better - updating an ugly looking list once in a blue moon or
 having apps use internal API?

 As said above: _if_ it turns out to be labour intensive - fine remove it.
 We've already spend more time researching than what the [expected]
 maintenance for 1 year would be ;-)
>>>
>>> It all depends on how you view scope and probability.
>>>
>>> I don't agree my concern is preemptive: we've just shipped a beta
>>> where 'make check' fails on a few architectures, and the proposed fix
>>> (adding several symbols to the platform list) will still fail on
>>> architectures Debian ships on, until it also adds _fbss, _fdata and
>>> _ftext. I think my suggestion is reasonable: it fixes the very real
>>> problem, in a way which avoids expanding the scope of our ABI checker
>>> to include the implementation details of every linker/architecture
>>> pair that people use Wayland on. The cost of this is the probability
>>> that someone manages to add a new underscored symbol marked with
>>> WL_EXPORT into a library which is currently 105 LoC, and have no-one
>>> notice that in review. I think that's a reasonable tradeoff.
>>
>> I'm inclined to agree with Daniel here.
>>
>> Someone managing to sneak a WL_EXPORT on a symbol starting with a _ past
>> review seems reasonably unlikely.
>>
>> And after reading this thread I'm still not entirely sure we have a
>> complete list of what should be on that whitelist, and there are only a
>> limited number of systems I can test build on right now.
>>
> Sharing a few ideas that are less obvious. Having the partial list
> allows you to:
>  - see active developers/users - be that by merging their patches or
> skimming through the report

I don't imagine we'll be adding new API to that library frequently
enough to mine that data productively.

>  - have contact point for people with unusual platforms/setups

By defaulting to a broken build state for unusual platforms, do you
really think we'll attract many contact points?

>  - is fairly trivial newbie task ;-)

I don't think we should be going out of our way to generate problems for
newbies to solve.  There are plenty of worthwhile tasks for new
participants already.

I've confirmed that we once again pass make check on arm (something I
really should've done prior to the beta release.  sigh.) and have pushed
this patch.

Thanks,
Derek

> 
> -Emil
> 

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Emil Velikov]

On 20 March 2018 at 14:50, Derek Foreman  wrote:
> On 2018-03-20 07:11 AM, Daniel Stone wrote:
>> On 20 March 2018 at 11:55, Emil Velikov  wrote:
>>> On 20 March 2018 at 11:46, Daniel Stone  wrote:
 Sure. As on IRC though, we definitely need to add at least _ftext for
 MIPS anyway:
 https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74

 And probably some more for ARM toolchains using other linkers:
 http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html

 Doing a quick check across all the architectures on Debian shows that
 your updated list is also missing _fbss, _fdata, and _ftext. So we'd
 need to respin for those as well, but I think at this point the
 inconvenience of maintaining a list of every linker's implementation
 detail on every platform, outweighs the risk of an exported
 underscore-prefixed symbol slipping through review.
>>>
>>> I think you're preemptively worried about this. Plus there's no need
>>> to add every symbol a search could find.
>>>
>>> Even then, ask yourself the question:
>>> Which is better - updating an ugly looking list once in a blue moon or
>>> having apps use internal API?
>>>
>>> As said above: _if_ it turns out to be labour intensive - fine remove it.
>>> We've already spend more time researching than what the [expected]
>>> maintenance for 1 year would be ;-)
>>
>> It all depends on how you view scope and probability.
>>
>> I don't agree my concern is preemptive: we've just shipped a beta
>> where 'make check' fails on a few architectures, and the proposed fix
>> (adding several symbols to the platform list) will still fail on
>> architectures Debian ships on, until it also adds _fbss, _fdata and
>> _ftext. I think my suggestion is reasonable: it fixes the very real
>> problem, in a way which avoids expanding the scope of our ABI checker
>> to include the implementation details of every linker/architecture
>> pair that people use Wayland on. The cost of this is the probability
>> that someone manages to add a new underscored symbol marked with
>> WL_EXPORT into a library which is currently 105 LoC, and have no-one
>> notice that in review. I think that's a reasonable tradeoff.
>
> I'm inclined to agree with Daniel here.
>
> Someone managing to sneak a WL_EXPORT on a symbol starting with a _ past
> review seems reasonably unlikely.
>
> And after reading this thread I'm still not entirely sure we have a
> complete list of what should be on that whitelist, and there are only a
> limited number of systems I can test build on right now.
>
Sharing a few ideas that are less obvious. Having the partial list
allows you to:
 - see active developers/users - be that by merging their patches or
skimming through the report
 - have contact point for people with unusual platforms/setups
 - is fairly trivial newbie task ;-)

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Derek Foreman]

On 2018-03-20 07:11 AM, Daniel Stone wrote:
> On 20 March 2018 at 11:55, Emil Velikov  wrote:
>> On 20 March 2018 at 11:46, Daniel Stone  wrote:
>>> Sure. As on IRC though, we definitely need to add at least _ftext for
>>> MIPS anyway:
>>> https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74
>>>
>>> And probably some more for ARM toolchains using other linkers:
>>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html
>>>
>>> Doing a quick check across all the architectures on Debian shows that
>>> your updated list is also missing _fbss, _fdata, and _ftext. So we'd
>>> need to respin for those as well, but I think at this point the
>>> inconvenience of maintaining a list of every linker's implementation
>>> detail on every platform, outweighs the risk of an exported
>>> underscore-prefixed symbol slipping through review.
>>
>> I think you're preemptively worried about this. Plus there's no need
>> to add every symbol a search could find.
>>
>> Even then, ask yourself the question:
>> Which is better - updating an ugly looking list once in a blue moon or
>> having apps use internal API?
>>
>> As said above: _if_ it turns out to be labour intensive - fine remove it.
>> We've already spend more time researching than what the [expected]
>> maintenance for 1 year would be ;-)
> 
> It all depends on how you view scope and probability.
> 
> I don't agree my concern is preemptive: we've just shipped a beta
> where 'make check' fails on a few architectures, and the proposed fix
> (adding several symbols to the platform list) will still fail on
> architectures Debian ships on, until it also adds _fbss, _fdata and
> _ftext. I think my suggestion is reasonable: it fixes the very real
> problem, in a way which avoids expanding the scope of our ABI checker
> to include the implementation details of every linker/architecture
> pair that people use Wayland on. The cost of this is the probability
> that someone manages to add a new underscored symbol marked with
> WL_EXPORT into a library which is currently 105 LoC, and have no-one
> notice that in review. I think that's a reasonable tradeoff.

I'm inclined to agree with Daniel here.

Someone managing to sneak a WL_EXPORT on a symbol starting with a _ past
review seems reasonably unlikely.

And after reading this thread I'm still not entirely sure we have a
complete list of what should be on that whitelist, and there are only a
limited number of systems I can test build on right now.

Reviewed-by: Derek Foreman 

> Cheers,
> Daniel
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> 

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Emil Velikov]

On 20 March 2018 at 12:11, Daniel Stone  wrote:
> On 20 March 2018 at 11:55, Emil Velikov  wrote:
>> On 20 March 2018 at 11:46, Daniel Stone  wrote:
>>> Sure. As on IRC though, we definitely need to add at least _ftext for
>>> MIPS anyway:
>>> https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74
>>>
>>> And probably some more for ARM toolchains using other linkers:
>>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html
>>>
>>> Doing a quick check across all the architectures on Debian shows that
>>> your updated list is also missing _fbss, _fdata, and _ftext. So we'd
>>> need to respin for those as well, but I think at this point the
>>> inconvenience of maintaining a list of every linker's implementation
>>> detail on every platform, outweighs the risk of an exported
>>> underscore-prefixed symbol slipping through review.
>>
>> I think you're preemptively worried about this. Plus there's no need
>> to add every symbol a search could find.
>>
>> Even then, ask yourself the question:
>> Which is better - updating an ugly looking list once in a blue moon or
>> having apps use internal API?
>>
>> As said above: _if_ it turns out to be labour intensive - fine remove it.
>> We've already spend more time researching than what the [expected]
>> maintenance for 1 year would be ;-)
>
> It all depends on how you view scope and probability.
>
> I don't agree my concern is preemptive: we've just shipped a beta
> where 'make check' fails on a few architectures, and the proposed fix
> (adding several symbols to the platform list) will still fail on
> architectures Debian ships on, until it also adds _fbss, _fdata and
> _ftext. I think my suggestion is reasonable: it fixes the very real
> problem, in a way which avoids expanding the scope of our ABI checker
> to include the implementation details of every linker/architecture
> pair that people use Wayland on. The cost of this is the probability
> that someone manages to add a new underscored symbol marked with
> WL_EXPORT into a library which is currently 105 LoC, and have no-one
> notice that in review. I think that's a reasonable tradeoff.
>
Right, hopefully there won't be any internals leaked/used by other projects.
I reserve myself the right of the lovely "I told you so".

Feel free to continue with whichever solution you feel comfortable with.

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Daniel Stone]

On 20 March 2018 at 11:55, Emil Velikov  wrote:
> On 20 March 2018 at 11:46, Daniel Stone  wrote:
>> Sure. As on IRC though, we definitely need to add at least _ftext for
>> MIPS anyway:
>> https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74
>>
>> And probably some more for ARM toolchains using other linkers:
>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html
>>
>> Doing a quick check across all the architectures on Debian shows that
>> your updated list is also missing _fbss, _fdata, and _ftext. So we'd
>> need to respin for those as well, but I think at this point the
>> inconvenience of maintaining a list of every linker's implementation
>> detail on every platform, outweighs the risk of an exported
>> underscore-prefixed symbol slipping through review.
>
> I think you're preemptively worried about this. Plus there's no need
> to add every symbol a search could find.
>
> Even then, ask yourself the question:
> Which is better - updating an ugly looking list once in a blue moon or
> having apps use internal API?
>
> As said above: _if_ it turns out to be labour intensive - fine remove it.
> We've already spend more time researching than what the [expected]
> maintenance for 1 year would be ;-)

It all depends on how you view scope and probability.

I don't agree my concern is preemptive: we've just shipped a beta
where 'make check' fails on a few architectures, and the proposed fix
(adding several symbols to the platform list) will still fail on
architectures Debian ships on, until it also adds _fbss, _fdata and
_ftext. I think my suggestion is reasonable: it fixes the very real
problem, in a way which avoids expanding the scope of our ABI checker
to include the implementation details of every linker/architecture
pair that people use Wayland on. The cost of this is the probability
that someone manages to add a new underscored symbol marked with
WL_EXPORT into a library which is currently 105 LoC, and have no-one
notice that in review. I think that's a reasonable tradeoff.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Emil Velikov]

On 20 March 2018 at 11:46, Daniel Stone  wrote:
> Hi Emil,
>
> On 20 March 2018 at 11:30, Emil Velikov  wrote:
>> On 20 March 2018 at 11:01, Daniel Stone  wrote:
>>> Rather than a hard-coded list of platform symbols, just ignore anything
>>> prefaced with an underscore. This fixes breakage on ARM, which declares
>>> several slightly different platform symbols to x86.
>>
>> FWIW I've explicitly opted against this kind of heuristics for a few reasons:
>>
>>  - projects use single/double underscored symbols for internal API
>>  - other projects have been using such symbols knowingly that it's internal 
>> API
>> Last but not least
>>  - the symbols exposed by the C runtime should stay stable
>>
>> In other words - prevents others from going silly things, only the
>> list might need 1-2 updates.
>> If the latter turns out to be false we can nuke it.
>
> Sure. As on IRC though, we definitely need to add at least _ftext for
> MIPS anyway:
> https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74
>
> And probably some more for ARM toolchains using other linkers:
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html
>
> Doing a quick check across all the architectures on Debian shows that
> your updated list is also missing _fbss, _fdata, and _ftext. So we'd
> need to respin for those as well, but I think at this point the
> inconvenience of maintaining a list of every linker's implementation
> detail on every platform, outweighs the risk of an exported
> underscore-prefixed symbol slipping through review.
>
I think you're preemptively worried about this. Plus there's no need
to add every symbol a search could find.

Even then, ask yourself the question:
Which is better - updating an ugly looking list once in a blue moon or
having apps use internal API?

As said above: _if_ it turns out to be labour intensive - fine remove it.
We've already spend more time researching than what the [expected]
maintenance for 1 year would be ;-)

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Daniel Stone]

Hi Emil,

On 20 March 2018 at 11:30, Emil Velikov  wrote:
> On 20 March 2018 at 11:01, Daniel Stone  wrote:
>> Rather than a hard-coded list of platform symbols, just ignore anything
>> prefaced with an underscore. This fixes breakage on ARM, which declares
>> several slightly different platform symbols to x86.
>
> FWIW I've explicitly opted against this kind of heuristics for a few reasons:
>
>  - projects use single/double underscored symbols for internal API
>  - other projects have been using such symbols knowingly that it's internal 
> API
> Last but not least
>  - the symbols exposed by the C runtime should stay stable
>
> In other words - prevents others from going silly things, only the
> list might need 1-2 updates.
> If the latter turns out to be false we can nuke it.

Sure. As on IRC though, we definitely need to add at least _ftext for
MIPS anyway:
https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74

And probably some more for ARM toolchains using other linkers:
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html

Doing a quick check across all the architectures on Debian shows that
your updated list is also missing _fbss, _fdata, and _ftext. So we'd
need to respin for those as well, but I think at this point the
inconvenience of maintaining a list of every linker's implementation
detail on every platform, outweighs the risk of an exported
underscore-prefixed symbol slipping through review.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Emil Velikov]

On 20 March 2018 at 11:01, Daniel Stone  wrote:
> Rather than a hard-coded list of platform symbols, just ignore anything
> prefaced with an underscore. This fixes breakage on ARM, which declares
> several slightly different platform symbols to x86.
>
FWIW I've explicitly opted against this kind of heuristics for a few reasons:

 - projects use single/double underscored symbols for internal API
 - other projects have been using such symbols knowingly that it's internal API
Last but not least
 - the symbols exposed by the C runtime should stay stable

In other words - prevents others from going silly things, only the
list might need 1-2 updates.
If the latter turns out to be false we can nuke it.

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Daniel Stone]

Rather than a hard-coded list of platform symbols, just ignore anything
prefaced with an underscore. This fixes breakage on ARM, which declares
several slightly different platform symbols to x86.

Signed-off-by: Daniel Stone 
Fixes: 21b1f22eb056 ("wayland-egl: enhance the symbol test")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105620
Cc: Emil Velikov 
---
 egl/wayland-egl-symbols-check | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
index 70fe1f4c..d04fd042 100755
--- a/egl/wayland-egl-symbols-check
+++ b/egl/wayland-egl-symbols-check
@@ -16,14 +16,6 @@ fi
 
 AVAIL_FUNCS="$($NM -D --format=bsd --defined-only $LIB | awk '{print $3}')"
 
-# Platform specific symbols.
-PLAT_FUNCS="__bss_start
-_edata
-_end
-_fini
-_init
-"
-
 # Official ABI, taken from the header.
 REQ_FUNCS="wl_egl_window_resize
 wl_egl_window_create
@@ -32,8 +24,8 @@ wl_egl_window_get_attached_size
 "
 
 NEW_ABI=$(echo "$AVAIL_FUNCS" | while read func; do
+echo "$func" | grep -q "^_" && continue
 echo "$REQ_FUNCS" | grep -q "^$func$" && continue
-echo "$PLAT_FUNCS" | grep -q "^$func$" && continue
 
 echo $func
 done)
-- 
2.16.2

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel