On 17.08.2016 02:18, Harlan Stenn wrote:
On 8/16/16 1:24 PM, Robert Munteanu wrote:
Hi,
Hopefully this is the right channel for such a patch. I have a minor enhancement to submit for the antispam plugin
http://hg.dovecot.org/dovecot-antispam-plugin
It adds minimal error checking for the sendmail_binary, otherwise the reported error in case of a missing binary or one with missing permissions is generic and not useful.
Thanks,
Robert Robert, I like that you did this.
Beyond that and without even looking at the actual code, I'm curious why you:
- if (access(cfg->binary, F_OK) == -1)
- {
- mail_storage_set_error(storage, MAIL_ERROR_TEMP, "mail_sendmail file does not exist");
instead of finding a way to include the value of cfg->binary in the error message string.
This might not be needed if it's really obvious from the config file what the path to the executable is, but if there is any doubt it might be friendlier to show the exact path with the problem. I'd also be inclined to show the decoded value of errno instead of assuming that 'mail_sendmail file does not exist'.
Perhaps something along the lines of:
"access(%s, F_OK) failed: %m", cfg->binary
if that makes sense.
H Hi!
Thank you for your patch, we'll take it under consideration!
Aki