Re: libcbor v0.10.0

2022-12-30 Thread Stuart Henderson
On 2022/12/30 02:06, Theo Buehler wrote:
> I understand that it is a libcbor major bump. Why is the libfido2 bump
> needed?

We can run into a problem with bumps with inter-library dependencies
in base. If software from packages uses functions from both libraries,
if you _don't_ bump major for libfido2, it will pull in the new fido2
lib (using new cbor abi), and depending on what ld.so decides, one or
other version of libcbor, that can't satisfy abi requirements of bith
new libfido2 and old binary.

It resolves itself when new packages are installed, but that takes time.

Now, I don't know if that actually happens with packages using
fido2/cbor; could be that they only want libfido2 functions and don't
use libcbor functions themselves, in which case there's no problem. It
happens very often when X libraries are bumped, but they're used by more
packages and there's higher likelihood of functions from both libraries
actually being called (or structs being passed around and potentially
copied incomplete, etc).



Re: libcbor v0.10.0

2022-12-29 Thread Theo Buehler
On Fri, Dec 30, 2022 at 12:41:59PM +1100, Damien Miller wrote:
> On Fri, 30 Dec 2022, Theo Buehler wrote:
> 
> > On Fri, Dec 30, 2022 at 10:09:16AM +1100, Damien Miller wrote:
> > > This updates libcbor to upstream version v.0.10.0. This version includes
> > > clang15 header fixes and fixes a few memory leaks. Full release notes
> > > are at https://github.com/PJK/libcbor/releases/tag/v0.10.0
> > 
> > I understand that it is a libcbor major bump. Why is the libfido2 bump
> > needed?
> 
> Caution - some of the libcbor changes were listed as "breaking" in the
> release notes. I took this to mean an ABI bump. libfido2 depends on
> libcbor, so I figured that bumping it would avoid any possibility of
> inconsistency between them at link time. Too much?

There are numerous ABI breaks in libcbor that do require a major bump:
size_t -> uint64_t in public API, enum values added and removed, ...

It's thus perfectly reasonable to bump libfido2. I was just wondering
whether there was a more specific reason since I couldn't immediately
spot anything. This bump is cheap, by all means keep it.

> > The CBOR_CUSTOM_ALLOC deprecation and the fact that cbor_set_allocs() is
> > now exposed is a bit disappointing.
> > 
> > https://github.com/PJK/libcbor/pull/237
> > 
> > Other than that the diff looks good to me, build tested on sparc64, so
> > gcc-archs should be fine, too.
> 
> That's reasonable. I can chop those out:

Yes, I think that makes sense and isn't super intrusive. I'd prefer this
version.

(It's a bit strange to write

  "Will keep this PR open for a few weeks to give people a chance to object."

and then turn around, merge it and cut a release containing it.)



Re: libcbor v0.10.0

2022-12-29 Thread Damien Miller
On Fri, 30 Dec 2022, Theo Buehler wrote:

> On Fri, Dec 30, 2022 at 10:09:16AM +1100, Damien Miller wrote:
> > This updates libcbor to upstream version v.0.10.0. This version includes
> > clang15 header fixes and fixes a few memory leaks. Full release notes
> > are at https://github.com/PJK/libcbor/releases/tag/v0.10.0
> 
> I understand that it is a libcbor major bump. Why is the libfido2 bump
> needed?

Caution - some of the libcbor changes were listed as "breaking" in the
release notes. I took this to mean an ABI bump. libfido2 depends on
libcbor, so I figured that bumping it would avoid any possibility of
inconsistency between them at link time. Too much?

> The CBOR_CUSTOM_ALLOC deprecation and the fact that cbor_set_allocs() is
> now exposed is a bit disappointing.
> 
> https://github.com/PJK/libcbor/pull/237
> 
> Other than that the diff looks good to me, build tested on sparc64, so
> gcc-archs should be fine, too.

That's reasonable. I can chop those out:


Index: Makefile
===
RCS file: /cvs/src/lib/libcbor/Makefile,v
retrieving revision 1.3
diff -u -p -r1.3 Makefile
--- Makefile3 Aug 2020 02:34:31 -   1.3
+++ Makefile30 Dec 2022 01:37:31 -
@@ -3,6 +3,8 @@
 .PATH: ${.CURDIR}/src ${.CURDIR}/src/cbor ${.CURDIR}/src/cbor/internal
 
 CFLAGS+= -I${.CURDIR}/src -DHAVE_ENDIAN_H -std=c99
+# We don't support custom allocators.
+CFLAGS+= -D_cbor_malloc=malloc -D_cbor_realloc=realloc -D_cbor_free=free
 
 LIB=   cbor
 SRCS=  cbor.c

cbor/common.h now looks like this:

+#if 0 /* custom allocators are not supported on OpenBSD */
+typedef void *(*_cbor_malloc_t)(size_t);
+typedef void *(*_cbor_realloc_t)(void *, size_t);
+typedef void (*_cbor_free_t)(void *);
+
+CBOR_EXPORT extern _cbor_malloc_t _cbor_malloc;
+CBOR_EXPORT extern _cbor_realloc_t _cbor_realloc;
+CBOR_EXPORT extern _cbor_free_t _cbor_free;
+#endif

...

+#if 0 /* not on OpenBSD */
+CBOR_EXPORT void cbor_set_allocs(_cbor_malloc_t custom_malloc,
+ _cbor_realloc_t custom_realloc,
+ _cbor_free_t custom_free);
 #endif






Re: libcbor v0.10.0

2022-12-29 Thread Theo Buehler
On Fri, Dec 30, 2022 at 10:09:16AM +1100, Damien Miller wrote:
> This updates libcbor to upstream version v.0.10.0. This version includes
> clang15 header fixes and fixes a few memory leaks. Full release notes
> are at https://github.com/PJK/libcbor/releases/tag/v0.10.0

I understand that it is a libcbor major bump. Why is the libfido2 bump
needed?

The CBOR_CUSTOM_ALLOC deprecation and the fact that cbor_set_allocs() is
now exposed is a bit disappointing.

https://github.com/PJK/libcbor/pull/237

Other than that the diff looks good to me, build tested on sparc64, so
gcc-archs should be fine, too.

ok tb