On 24 Jan 2016 19:44, "Marco Pivetta" <ocram...@gmail.com> wrote:
>
> On 24 January 2016 at 20:34, Jakub Zelenka <bu...@php.net> wrote:
>>
>> On 24 Jan 2016 14:49, "Marco Pivetta"
>> >
>> > The BC breaks may happen in future, given the number of added
parameters. You even designed your additional parameters to avoid BC
breaks: what will happen next time someone needs additional functionality?
>>
>> I'm not aware about any new functionality that could be added here in
the near future and even if there was any, new parameters can still be
added. I'm pretty sure that once this happens we will have already named
params support which should address all your concerns. I actualy think that
with the named params this will be much better API change than all other
propasals.
>
>
> Exactly: like anyone else, you are not yet aware about it. What happens
in 5 years?

Of course I don't know what happens in 5 years but even if we needed to add
new parameters, it wouldn't be a BC break.

>>
>> I'm sorry but I don't think we should add any objects here. OpenSSL ext
is purely procedural and objects wouldn't fit. It would also require some
code reordering and clean up.
>
> Doesn't really matter if you design it with an object or with optional
parameters, or even with an array of `$options`, as long as you can enforce
correct types passed in, and the order of the default parameter values
doesn't become a mess (as it currently is becoming).

I don't think that array would be an option as it's not realy nice when a
tag would be returned into it (tag is a ref and filled after encryption)...
the correct types are already enforced in the current implementation
(warning is triggered if they are not correct). About the order, the only
default is the last paremeter so I don't see any issue here either. As I
said I really think that we will get named params before there is any need
to add another param. After that the current API will be really good (just
look how many parameters are often in Python libraries).

>> >>
>> >> That being said I know that the current proposal is not the nicest
but I think it fits to the current openssl ext API which is not the nicest
either.
>> >
>> > This is indeed an adapter: if OpenSSL sucks, then don't jump off the
bridge as well ;-)
>>
>> I have never said that OpenSSL sucks. :) It's a great library written by
one of the smartest people in the world. I meant that our openssl extension
doesn't have the nicest API. However the API is more or less done in the
same style which we shouldn't change IMHO.
>
> I write loads of stuff that may or may not suck from a consumer
perspective. I am blaming the API, not the developer, let's not bring it to
a personal level.

Apology if it seemed that I'm bringing it on personal level. That wasn't my
intention. What I was trying to say is that I think we should try to keep
the extension consistent. I think that your idea is good in general but it
doesn't fit to the openssl ext in my opinion.
>
> We are designing (changing, eek!) an API here: the fact that it may or
not reflect how OpenSSL itself exposes it is totally irrelevant.

The OpenSSL libcrypto API was never the point here. I might have been a bit
unclear. If so, then I apologise. I actually meant other PHP functions from
openssl core extension (those starting with openssl_). There are no other
objects in the extension, just resources and functions with many
parameters. Some of them also using refs for out params. I also think that
it's more conforming with the openssl_encrypt and openssl_decrypt that got
iv parameter at some later point. We don't have a new pair of functions
with and without iv though...

Cheers

Jakub

Reply via email to