On Tuesday, Nov 4, 2003, at 21:10 Europe/Helsinki, Matthew Reimer wrote:
I think I've found the bug. The attached patch fixes the problem for me. The first problem is that the wrong value is being returned by message_send() because ret is getting set to the wrong value (see above or patch). The second problem is that in the special case where physical_size == virtual_size (i.e., sendfile can be used) i_stream_skip(input, virtual_skip) is being called on the input stream. This is wrong because virtual_skip is supposed to indicate that \r was the last character seen and therefore \n needs to be sent before any of the input stream; but in the sendfile case this is not necessary.
I think a little refactoring of seek_partial(), message_send() and message_skip_virtual() might be helpful to clarify the code. The virtual_skip argument to message_send() is an off_t but is only used to indicate that \r was the last character read, so it would probably be more clear to add another argument cr_skipped (like message_skip_virtual()). Better might be to put all the partial cache stuff in message-send.c.
It actually was supposed to be skip byte count, not just CR-indicator. But you're right, it was used wrong in sendfile code path. There was one piece of code anymore which used the virtual_skip more than just for CR (BODY[HEADER]<..>) and it was easy to change. I think the attached patch should fix it properly.