[patch] Improved error checking for the dovecot-antispam-plugin

[Robert Munteanu]

Hi Harlan,

On Wed, Aug 17, 2016 at 2:18 AM, Harlan Stenn <harlan at pfcs.com> 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.

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



-- 
http://robert.muntea.nu/