[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] libpoke: Make all symbols hidden by default
From: |
Jose E. Marchesi |
Subject: |
Re: [PATCH] libpoke: Make all symbols hidden by default |
Date: |
Wed, 28 Oct 2020 14:14:58 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
> On Tue, Oct 27, 2020 at 12:28:26PM +0100, Jose E. Marchesi wrote:
>>
>> Hi Mohammad!
>>
>> Thank you for this patch! It certainly feels good to get rid of all
>> these __attribute__ ((visibility ("hidden"))) :)
>>
>> > 2020-10-26 Mohammad-Reza Nabipoor <m.nabipoor@yahoo.com>
>> >
>> > * bootstrap.conf: Add `lib-symbol-visibility` module to `libpoke`.
>> > * libpoke/Makefile.am: Add CPP flags to hide symbols by default.
>> > * libpoke/libpoke.h: Define `LIBPOKE_API` macro. Mark all public
>> > functions with `LIBPOKE_API`.
>> > * libpoke/ios.h: Remove `__attribute__ ((visibility ("hidden")))`.
>> > * libpoke/pkl-asm.h: Likewise.
>> > * libpoke/pkl-ast.h: Likewise.
>> > * libpoke/pkl-diag.h: Likewise.
>> > * libpoke/pkl-env.h: Likewise.
>> > * libpoke/pkl-parser.h: Likewise.
>> > * libpoke/pkl-pass.h: Likewise.
>> > * libpoke/pkl.h: Likewise.
>> > * libpoke/pkl-alloc.h: Likewise.
>> > * libpoke/pvm-program.h: Likewise.
>> > * libpoke/pvm-val.h: Likewise.
>> > * libpoke/pvm.h: Likewise.
>>
>> The changes above are OK for master. Feel free to put them in their own
>> commit and push them.
>>
>
>
> Pushed.
>
>
>> > * etc/libpoke-public-functions: New file. Name of all public functions
>> > of `libpoke` per line.
>>
>> Let's talk a bit about the best way to validate the interface, before
>> committing to maintain something like etc/libpoke-public-functions,
>>
>> It seems to me that what constituted the API can the reliably be
>> automatically determined from libpoke.h by looking at the places where
>> LIBPOKE_API is used. No need to maintain a separated list by hand.
>>
>> At some point I imagine we will be using version files anyway, where we
>> will have to list the exported symbols explicitly anyway. But I say
>> let's cross that bridge if/when we reach that river. We don't need that
>> extra complexity now that we aren't even released!
>>
>
>
> OK. You're right.
>
> I've considered the automatic extraction of API; I've dropped this idea
> because
> I thought that check doesn't worth the effort and will not catch any bugs.
> Checking that every public function in `libpoke.h`, also exists inside
> the `libpoke.so` is unable to catch the mistakes.
>
> I think we're in a good state now and maybe we don't need any kind of
> automatic
> tests of exported symbols.
> Maybe having a list of public functions is kind of over-engineering :)
> I think having more tests of `libpoke` public functions is much more useful.
>
>
>> Using poke to look in the shared object for the global symbols is of
>> course much more preferable than using readelf! ;)
>
>
> :D
>
>
>>
>> > diff --git a/DEV-NEWS b/DEV-NEWS
>> > index 709ccc0e..ed9c5dfd 100644
>> > --- a/DEV-NEWS
>> > +++ b/DEV-NEWS
>> > @@ -7,3 +7,8 @@ poke.
>> > are permitted in any medium without royalty provided the copyright
>> > notice and this notice are preserved.
>> >
>> > +2020-10-26
>> > + * libpoke: Make all symbols hidden by default.
>> > + * etc/libpoke-public-functions: Sorted list of all public functions of
>> > + `libpoke`. Update this file if you add/remove an API function (in
>> > + `libpoke/libpoke.h` header file).
>>
>> I would keep the format of DEV-NEWS simpler, something readable at a
>> glance.
>>
>> What about:
>>
>> 2020-10-26
>> . libpoke is now built with all symbols hidden by default. Public
>> symbols in libpoke.h are explicitly marked as such.
>
>
> Thanks for the suggestion :+1:
> I've changed the format.
Thank you.