I have recently upgraded my SpamAssassin-based product to use SA 3.0-rc2. Generally, I think 3.0 is excellent; congratulations to the developers. I have a few comments:

- A bug was introduced between rc1 and rc2. In BayesStore/DBM.pm, the calls to tie were changed to store the return value. For example,

    $res = tie %new_toks, "DB_File", "${name}.new", O_RDWR|O_CREAT|O_EXCL,
          (oct ($main->{conf}->{bayes_file_mode}) & 0666);
    umask $umask;
    return 0 unless $res;

Later, %new_toks is untied and files are renamed. Storing $res, however, keeps a handle to the underlying file open, so the rename fails (at least on Windows). The fix is simple: after "return 0...", just add:

    undef $res;

- The plugin interface is a great idea; it allowed me to move code I previously had to integrate into each new version of SA out of the SA tree. As currently designed, however, plugin methods are effectively restricted to returning integers due to the comparison with INHIBIT_CALLBACKS. I suggest that as few assumptions as possible be made about plugin method return values, and we can just require methods to call inhibit_callbacks() if they want to instead of returning INHIBIT_CALLBACKS.

- I suggest adding a plugin callback to PerMsgStatus::_get_tag, so plugins can define their own report tags.

- I also suggest adding plugin callbacks (a) to get_report(), so plugins can change the report format, and (b) to report_safe() etc., so plugins can modify the rewritten message. The implementation of these is less clear than the _get_tag callback, though.

Thanks again for all the work!

Barry



Reply via email to