Timo Sirainen wrote:
I downloaded it when you first mentioned it, but looks like I never got around to actually looking at it. Yes, something like this would be nice..
One thing at least that should be changed is to configure it using virtual mailbox names instead of full mailbox paths. This isn't really possible with v1.0, but v1.1 should make it pretty easy.
You already told me about this last time, but I haven't yet come to read the source code of dovecot 1.1...
A few things about the code:
save_dest_mail = mail_alloc(ctx->transaction, MAIL_FETCH_PHYSICAL_SIZE, NULL);
Quota plugin wanted to know the message's physical size, but your plugin doesn't need it. So you could just use 0 instead of MAIL_FETCH_PHYSICAL_SIZE.
Thanks for pointing this.
I think write() can return partially written data if the other side isn't reading it fast enough. Using write_full() instead would anyway be better/safer.
I just reread my code, and I think my use of write looks safe, since only the amount that was correctly written is skipped with i_stream_skip. Do you think I'm missing something?
Do you really need to wait for the executed process to finish? Since this is the only plugin currently creating child processes, I'd setup a SIGCHLD handler and waitpid() there to get rid of the zombie: lib_signals_set_handler(SIGCHLD, TRUE, chld_handler, NULL);
Waiting is required if we want the append to fail if the command fails. I guess it should better be a configurable option, don't you think so?
Multiple commands are now processed sequentially. I don't know if there's real need for multiple commands, but it would be faster to read the message input just once and send it to all pipes in parallel.
I don't think there is real need for multiple commands, but I think it was easier for me to program things like this... ;-)
"return 0 * WEXITSTATUS(status);" returns always 0 :)
Hummm... I guess I should avoid drinking alcohol before I program... ;-) It probably should be "return WEXITSTATUS(status);"; I have no idea why I changed this in such a strange way...
I'd also leave stderr as-is for the child process so it could log errors, and for handling syscall failures use: i_fatal("dup2() failed: %m");
OK, point taken.
One question still: would you consider merging my plugin in dovecot if I ported it to 1.1?
Cheers,
Nicolas