Suggested changes to PurpleCipherOps and need for code review

[Tomasz Wasilczyk]

2013/5/1 Ethan Blanton <elb at pidgin.im>:
>> My suggestions:
>> - merge set_key() with set_key_with_len() (using ssize_t len instead
>> of size_t, if we don't want to provide length)
>> - add get_digest_size(), if we don't want to guess buffer length for digest()
>> - add (optional) length parameter for set_salt()
>
> All of this seems fine to me.  I'm not sure the lengths should be
> optional, but I'd have to see how you intend to derive them to be
> sure.  (Note that strlen() isn't really reasonable here.)

I'm also not sure about it. There are ciphers with constant
key/salt/whatever lengths (I guess, that current API was designed for
them), but it would be a good idea to double-check it here.

So, I'm in favor of making length parameters required.

>> - add output_size parameter to encrypt() and decrypt() to prevent
>> buffer overflow (outlen is used to store actual data length after
>> encryption/decryption)
>
> A common technique for this is to use outlen as an in/out parameter;
> it contains the buffer size at call time, and the bytes written at
> return time.  It may or may not be applicable here.

I was thinking of it, but I see two problems with it:
- API changes (turning outlen from only-out to in-out parameter)
without changing function definitions - its really easy to omit it
when updating a plugin from 2.x.y to 3.0.0 API
- it doesn't let specifying it by sizeof parameter, for example
encrypt(..., buff, sizeof(buff), &outlen)

Tomek