Re: Technically an API break
On 07/05/2020 16:02, Brian Smith wrote: > This kind of change might cause memory unsafety issues unless the > application is recompiled. At least, it's worth investigating that. > > On most platforms the ABI of a function that returns `void` and one that > returns `int` is the same, from the perspective of a caller that doesn't > expect or use the return value. I seem to vaguely remember in the past > that there was at least one common platform where that isn't true > though. Unfortunately I cannot remember which one it is. I also don't > remember if it is problematic to change from "int" to "void" or "void" > to "int" or both. > > Anyway, my point is that you should consider this an ABI-breaking > change, not just an API breaking one. Yes - thanks for that Brian. Actually though this change is targeted only at the master branch (which will become OpenSSL 3.0). That is a major release and is already ABI breaking - so recompilation is already a requirement. Actually as a major release we are allowed to be API breaking too, but we are trying to keep that to a minimum. Matt
Re: Technically an API break
Matt Caswell wrote: > PR11589 makes a change to the public API function > `SSL_set_record_padding_callback` to change its return type from void to > int: > > https://github.com/openssl/openssl/pull/11589 > > This is technically an API break - but it doesn't seem too serious. It's > possible, I suppose, that existing applications that use this will fail > to spot the error return since this function can now fail. The function > itself was only recently added (in 1.1.1), and I suspect real-world > usage is very small (or possibly nil). > > Is this considered ok? > This kind of change might cause memory unsafety issues unless the application is recompiled. At least, it's worth investigating that. On most platforms the ABI of a function that returns `void` and one that returns `int` is the same, from the perspective of a caller that doesn't expect or use the return value. I seem to vaguely remember in the past that there was at least one common platform where that isn't true though. Unfortunately I cannot remember which one it is. I also don't remember if it is problematic to change from "int" to "void" or "void" to "int" or both. Anyway, my point is that you should consider this an ABI-breaking change, not just an API breaking one. Cheers, Brian
Re: Technically an API break
On Thu, 07 May 2020 10:31:42 +0200, Tomas Mraz wrote: > > On Thu, 2020-05-07 at 09:24 +0100, Matt Caswell wrote: > > PR11589 makes a change to the public API function > > `SSL_set_record_padding_callback` to change its return type from void > > to > > int: > > > > https://github.com/openssl/openssl/pull/11589 > > > > This is technically an API break - but it doesn't seem too serious. > > It's > > possible, I suppose, that existing applications that use this will > > fail > > to spot the error return since this function can now fail. The > > function > > itself was only recently added (in 1.1.1), and I suspect real-world > > usage is very small (or possibly nil). > > > > Is this considered ok? > > I would say this is an acceptable thing if it is documented (which it > is in the PR). Especially because the error return can happen only when > the application sets up the SSL to use kernel TLS. I agree with this assessment. Cheers, Richard -- Richard Levitte levi...@openssl.org OpenSSL Project http://www.openssl.org/~levitte/
Re: Technically an API break
On Thu, 2020-05-07 at 09:24 +0100, Matt Caswell wrote: > PR11589 makes a change to the public API function > `SSL_set_record_padding_callback` to change its return type from void > to > int: > > https://github.com/openssl/openssl/pull/11589 > > This is technically an API break - but it doesn't seem too serious. > It's > possible, I suppose, that existing applications that use this will > fail > to spot the error return since this function can now fail. The > function > itself was only recently added (in 1.1.1), and I suspect real-world > usage is very small (or possibly nil). > > Is this considered ok? I would say this is an acceptable thing if it is documented (which it is in the PR). Especially because the error return can happen only when the application sets up the SSL to use kernel TLS. -- Tomáš Mráz No matter how far down the wrong road you've gone, turn back. Turkish proverb [You'll know whether the road is wrong if you carefully listen to your conscience.]
Technically an API break
PR11589 makes a change to the public API function `SSL_set_record_padding_callback` to change its return type from void to int: https://github.com/openssl/openssl/pull/11589 This is technically an API break - but it doesn't seem too serious. It's possible, I suppose, that existing applications that use this will fail to spot the error return since this function can now fail. The function itself was only recently added (in 1.1.1), and I suspect real-world usage is very small (or possibly nil). Is this considered ok? Matt
