[patch] Improved error checking for the dovecot-antispam-plugin
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
On 8/16/16 1:24 PM, Robert Munteanu wrote:
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 Harlan,
On Wed, Aug 17, 2016 at 2:18 AM, Harlan Stenn <harlan@pfcs.com> wrote:
Thanks for the review . I was not sure that it's OK to show the path to the script in an error message which will be shown to the user. But I have no issue in resending a new version of the patch with better error reporting, will do so in the following days.
Robert
(snip)
I've attached a second version of the patch, feel free to consider any of them for inclusion.
Thanks,
Robert
Robert,
First, thanks!
Second, I'm not a committer on the dovecot project. But I've written a lot of software where if an end user has a problem and either they want to know why or if they report it and ask for help, I've found it is MUCH better to have enough info in the message given to the user/logged somewhere. Something like:
"subroutine: open(%s) failed: %m"
It reduces our support load and gives us the information we need to quickly resolve issues.
Sent from my iPhone - please excuse brevity and typos
Hi,
Has this slipped off the radar or is it somehow not suitable for inclusion?
Thanks,
Robert
On Thu, Aug 18, 2016 at 6:16 PM, Robert Munteanu <robert.munteanu@gmail.com> wrote:
participants (3)
-
Aki Tuomi
-
Harlan Stenn
-
Robert Munteanu