Re: [PATCH] always export the symbols for the proc_service interface

Norbert Lange
2020-01-06T23:22:15Z

Am Mo., 6. Jan. 2020 um 17:07 Uhr schrieb Tom Tromey <tom@tromey.com>:
>
> >>>>> "Norbert" == Norbert Lange <nolange79@gmail.com> writes:
>
> Norbert> Compiling GDB with '-fvisibility=hidden' will remove the
> Norbert> symbols that should be exported.
> Norbert> This patch explicitly marks them as visible.
>
> Norbert> gdb/ChangeLog
>
> Norbert> PR build/24805
> Norbert> * gdbsupport/gdb_proc_service.h: push/pop visibility
> Norbert> attribute for exported functions.
>
> Thanks for the patch.
>
> I have one question and one comment.
>
> Norbert> +++ b/gdb/gdbsupport/gdb_proc_service.h
> Norbert> @@ -23,6 +23,11 @@
>
> Norbert> #ifdef HAVE_PROC_SERVICE_H
>
> Norbert> +/* ensure the symbols are public, even if the default is hidden */
>
> In the gdb style, comments should be complete sentences, which end in a
> period followed by 2 spaces. So this should read:
>
> /* Ensure the symbols are public, even if the default is hidden. */

Noted, am I required to redo the patch if that's the only change?

>
> Norbert> +#if __GNUC__ >= 4
> Norbert> +#pragma GCC visibility pop
> Norbert> +#endif
>
> Would it be better to just have this done a single time at the top and
> bottom of the file?

there are other headers included in between, so that's a bad idea
(and you could do attach function attributes instead for the second case,
see patch in bug report.)

I still dont get why the external header is supported at all, if the
implementation
needs to match 100% anyway (unlike where you just call functions from a header).

Norbert