confusing behavior of OTRL_FRAGMENT_SEND_ALL wrt. plaintext messages
When using OTRL_FRAGMENT_SEND_ALL, libotr handles outgoing unmodified plaintext messages confusingly.
With libotr 3 it was the caller's responsibility to send any messages or fragments after otrl_message_sending. There were three cases:
- otrl_message_sending returns an error. We should send nothing.
- otrl_message_sending returns success and sets its messagep argument. We should send *messagep via otrl_fragment_and_send.
- otrl_message_sending returns success and sets messagep to NULL. We should send the original message.
The OTRL_FRAGMENT_SEND_ALL path of libotr 4.0.0 handles 1 (as in not sending anything) and 2 but leaves case 3 to the caller.
This is understandable behaviour from the inside but confusing to the user (of libotr). I at least didn't expect a NULL value from an out parameter which one isn't otherwise interested in to signify a need for action. Moreover I mistook SEND_ALL to mean that libotr would take care of all sending.
Might I suggest the following clarifying addition to the documentation of otrl_message_sending? (Also, I think messagep was meant in the line above, please confirm.)
--- a/src/message.c +++ b/src/message.c @@ -192,7 +192,10 @@ void otrl_message_free(char *message) * Other fragmentation policies are OTRL_FRAGMENT_SEND_ALL, * OTRL_FRAGMENT_SEND_ALL_BUT_LAST, or OTRL_FRAGMENT_SEND_ALL_BUT_FIRST. In * these cases, the appropriate fragments will be automatically sent. For the - * last two policies, the remaining fragment will be passed in *original_msg. + * last two policies, the remaining fragment will be passed in *messagep. + * + * With OTRL_FRAGMENT_SEND_ALL, if the routine returns non-zero and sets + * *messagep to NULL, you must still send the original message. * * Call otrl_message_free(*messagep) if you don't need *messagep or when you're * done with it. */
(from redmine: created on 2014-04-18)