On 5/1/2022 1:28 PM, Michael Storz wrote:
Am 2022-05-01 18:22, schrieb Kevin A. McGrail:
Morning Hege,
This change worries me. What does the comment "let per figure it out
from the BOM" mean and does the change on the return undef change
that?
Worried that Perl Critic is complaining about something that was done
on purpose.
Do we have a good test case for this change? if not can we synthesize
a spample and add a test for it?
Regards,
KAM
On 5/1/2022 5:25 AM, h...@apache.org wrote:
Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Message/Node.pm
URL:http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Message/Node.pm?rev=1900446&r1=1900445&r2=1900446&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Message/Node.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Message/Node.pm Sun
May 1 09:25:11 2022
@@ -389,7 +389,7 @@ sub detect_utf16 {
# avoid scan if BOM present
if( $data =~ /^(?:\xff\xfe|\xfe\xff)/ ) {
dbg( "message: detect_utf16: found BOM" );
- return undef; # let perl figure it out from the BOM
+ return; # let perl figure it out from the BOM
}
Kevin,
using "return undef" on purpose means the subroutine is called in list
context and instead of an empty list you want undef.
grep -r detect_utf16 lib/Mail/ shows
lib/Mail/SpamAssassin/Message/Node.pm:sub detect_utf16 {
lib/Mail/SpamAssassin/Message/Node.pm: dbg( "message:
detect_utf16: found BOM" );
lib/Mail/SpamAssassin/Message/Node.pm: dbg( "message:
detect_utf16: UTF-16LE" );
lib/Mail/SpamAssassin/Message/Node.pm: dbg( "message:
detect_utf16: UTF-16BE" );
lib/Mail/SpamAssassin/Message/Node.pm: dbg( "message:
detect_utf16: Could not detect UTF-16 endianness" );
lib/Mail/SpamAssassin/Message/Node.pm: my $decoder = detect_utf16(
$_[0] );
which means detect_utf16 is called once in scalar context. That means
the change is correct.
Michael
Michael, Hege's change removed return undef; which I think might be
wanted in a few cases and that PerlCritic is a FP in this patch. You
seem to be agreeing that return undef is warranted which is why I raised
the issue here. Can you confirm that is what you are saying?
Regards,
KAM
--
Kevin A. McGrail
kmcgr...@apache.org
Member, Apache Software Foundation
Chair Emeritus Apache SpamAssassin Project
https://www.linkedin.com/in/kmcgrail - 703.798.0171
