[PATCH 1/2] libnftables.map: export new nft_ctx_{get,set}_optimize API

[PATCH 1/2] libnftables.map: export new nft_ctx_{get,set}_optimize API

[Sam James]

Without this, we're not explicitly saying this is part of the public
API.

This new API was added in 1.0.2 and is used by e.g. the main
nft binary. Noticed when fixing the version-script option
(separate patch) which picked up this problem when .map
was missing symbols (related to when symbol visibility
options get set).

Signed-off-by: Sam James <sam@gentoo.org>
---
 src/libnftables.map | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/libnftables.map b/src/libnftables.map
index a511dd78..f8cf05dc 100644
--- a/src/libnftables.map
+++ b/src/libnftables.map
@@ -32,4 +32,6 @@ LIBNFTABLES_2 {
 LIBNFTABLES_3 {
   nft_set_optimize;
   nft_get_optimize;
+  nft_ctx_set_optimize;
+  nft_ctx_get_optimize;
 } LIBNFTABLES_2;
-- 
2.35.1

[PATCH 2/2] build: explicitly pass --version-script to linker

[Sam James]

--version-script is a linker option, so let's use -Wl, so that
libtool handles it properly. It seems like the previous method gets silently
ignored with GNU libtool in some cases(?) and downstream in Gentoo,
we had to apply this change to make the build work with slibtool anyway.

But it's indeed correct in any case, so let's swap.

Signed-off-by: Sam James <sam@gentoo.org>
---
 src/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 4cfba0af..e96cee77 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -91,7 +91,7 @@ libparser_la_CFLAGS = ${AM_CFLAGS} \
 
 libnftables_la_LIBADD = ${LIBMNL_LIBS} ${LIBNFTNL_LIBS} libparser.la
 libnftables_la_LDFLAGS = -version-info ${libnftables_LIBVERSION} \
-			 --version-script=$(srcdir)/libnftables.map
+			 -Wl,--version-script=$(srcdir)/libnftables.map
 
 if BUILD_MINIGMP
 noinst_LTLIBRARIES += libminigmp.la
-- 
2.35.1

Re: [PATCH 1/2] libnftables.map: export new nft_ctx_{get,set}_optimize API

[Sam James]

[-- Attachment #1: Type: text/plain, Size: 3450 bytes --]



> On 24 Feb 2022, at 19:45, Sam James <sam@gentoo.org> wrote:
> 
> Without this, we're not explicitly saying this is part of the public
> API.
> 
> This new API was added in 1.0.2 and is used by e.g. the main
> nft binary. Noticed when fixing the version-script option
> (separate patch) which picked up this problem when .map
> was missing symbols (related to when symbol visibility
> options get set).
> 

Actually, I'm wondering if we need way more?

`abidiff` yields (between 1.0.1 and 1.0.2 for me):
```
 * 22 Removed functions:
 *
 *   [D] 'function int nft_ctx_add_include_path(nft_ctx*, const char*)'    {nft_ctx_add_include_path@@LIBNFTABLES_1}
 *   [D] 'function int nft_ctx_add_var(nft_ctx*, const char*)'    {nft_ctx_add_var@@LIBNFTABLES_2}
 *   [D] 'function int nft_ctx_buffer_error(nft_ctx*)'    {nft_ctx_buffer_error@@LIBNFTABLES_1}
 *   [D] 'function int nft_ctx_buffer_output(nft_ctx*)'    {nft_ctx_buffer_output@@LIBNFTABLES_1}
 *   [D] 'function void nft_ctx_clear_include_paths(nft_ctx*)'    {nft_ctx_clear_include_paths@@LIBNFTABLES_1}
 *   [D] 'function void nft_ctx_clear_vars(nft_ctx*)'    {nft_ctx_clear_vars@@LIBNFTABLES_2}
 *   [D] 'function void nft_ctx_free(nft_ctx*)'    {nft_ctx_free@@LIBNFTABLES_1}
 *   [D] 'function bool nft_ctx_get_dry_run(nft_ctx*)'    {nft_ctx_get_dry_run@@LIBNFTABLES_1}
 *   [D] 'function const char* nft_ctx_get_error_buffer(nft_ctx*)'    {nft_ctx_get_error_buffer@@LIBNFTABLES_1}
 *   [D] 'function const char* nft_ctx_get_output_buffer(nft_ctx*)'    {nft_ctx_get_output_buffer@@LIBNFTABLES_1}
 *   [D] 'function nft_ctx* nft_ctx_new(uint32_t)'    {nft_ctx_new@@LIBNFTABLES_1}
 *   [D] 'function unsigned int nft_ctx_output_get_debug(nft_ctx*)'    {nft_ctx_output_get_debug@@LIBNFTABLES_1}
 *   [D] 'function unsigned int nft_ctx_output_get_flags(nft_ctx*)'    {nft_ctx_output_get_flags@@LIBNFTABLES_1}
 *   [D] 'function void nft_ctx_output_set_debug(nft_ctx*, unsigned int)'    {nft_ctx_output_set_debug@@LIBNFTABLES_1}
 *   [D] 'function void nft_ctx_output_set_flags(nft_ctx*, unsigned int)'    {nft_ctx_output_set_flags@@LIBNFTABLES_1}
 *   [D] 'function void nft_ctx_set_dry_run(nft_ctx*, bool)'    {nft_ctx_set_dry_run@@LIBNFTABLES_1}
 *   [D] 'function FILE* nft_ctx_set_error(nft_ctx*, FILE*)'    {nft_ctx_set_error@@LIBNFTABLES_1}
 *   [D] 'function FILE* nft_ctx_set_output(nft_ctx*, FILE*)'    {nft_ctx_set_output@@LIBNFTABLES_1}
 *   [D] 'function int nft_ctx_unbuffer_error(nft_ctx*)'    {nft_ctx_unbuffer_error@@LIBNFTABLES_1}
 *   [D] 'function int nft_ctx_unbuffer_output(nft_ctx*)'    {nft_ctx_unbuffer_output@@LIBNFTABLES_1}
 *   [D] 'function int nft_run_cmd_from_buffer(nft_ctx*, const char*)'    {nft_run_cmd_from_buffer@@LIBNFTABLES_1}
 *   [D] 'function int nft_run_cmd_from_filename(nft_ctx*, const char*)'    {nft_run_cmd_from_filename@@LIBNFTABLES_1}
```

AFAIK none of these are internal so we I think want the whole context API.

> Signed-off-by: Sam James <sam@gentoo.org>
> ---
> src/libnftables.map | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/src/libnftables.map b/src/libnftables.map
> index a511dd78..f8cf05dc 100644
> --- a/src/libnftables.map
> +++ b/src/libnftables.map
> @@ -32,4 +32,6 @@ LIBNFTABLES_2 {
> LIBNFTABLES_3 {
>   nft_set_optimize;
>   nft_get_optimize;
> +  nft_ctx_set_optimize;
> +  nft_ctx_get_optimize;
> } LIBNFTABLES_2;
> --
> 2.35.1
> 

Best,
sam

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

Re: [PATCH 1/2] libnftables.map: export new nft_ctx_{get,set}_optimize API

[Sam James]

[-- Attachment #1: Type: text/plain, Size: 760 bytes --]



> On 24 Feb 2022, at 21:39, Sam James <sam@gentoo.org> wrote:
> 
> 
> 
>> On 24 Feb 2022, at 19:45, Sam James <sam@gentoo.org> wrote:
>> 
>> Without this, we're not explicitly saying this is part of the public
>> API.
>> 
>> This new API was added in 1.0.2 and is used by e.g. the main
>> nft binary. Noticed when fixing the version-script option
>> (separate patch) which picked up this problem when .map
>> was missing symbols (related to when symbol visibility
>> options get set).
>> 
> 
> Actually, I'm wondering if we need way more?
> [snip]
> AFAIK none of these are internal so we I think want the whole context API.
> 

Sorry, disregard this last email. The patches stand as per the original series :)

Bit frazzled today.

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

Re: [PATCH 1/2] libnftables.map: export new nft_ctx_{get,set}_optimize API

[Pablo Neira Ayuso]

On Thu, Feb 24, 2022 at 07:45:42PM +0000, Sam James wrote:
> Without this, we're not explicitly saying this is part of the public
> API.
> 
> This new API was added in 1.0.2 and is used by e.g. the main
> nft binary. Noticed when fixing the version-script option
> (separate patch) which picked up this problem when .map
> was missing symbols (related to when symbol visibility
> options get set).

Applied.

> Signed-off-by: Sam James <sam@gentoo.org>
> ---
>  src/libnftables.map | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/libnftables.map b/src/libnftables.map
> index a511dd78..f8cf05dc 100644
> --- a/src/libnftables.map
> +++ b/src/libnftables.map
> @@ -32,4 +32,6 @@ LIBNFTABLES_2 {
>  LIBNFTABLES_3 {
>    nft_set_optimize;
>    nft_get_optimize;

Removed the incorrect symbol name (missing _ctx_ infix)

> +  nft_ctx_set_optimize;
> +  nft_ctx_get_optimize;
>  } LIBNFTABLES_2;
> -- 
> 2.35.1
> 

Re: [PATCH 2/2] build: explicitly pass --version-script to linker

[Pablo Neira Ayuso]

On Thu, Feb 24, 2022 at 07:45:43PM +0000, Sam James wrote:
> --version-script is a linker option, so let's use -Wl, so that
> libtool handles it properly. It seems like the previous method gets silently
> ignored with GNU libtool in some cases(?) and downstream in Gentoo,
> we had to apply this change to make the build work with slibtool anyway.
> 
> But it's indeed correct in any case, so let's swap.

Also applied, thanks