Am 2022-05-01 20:02, schrieb Kevin A. McGrail:
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, the change from 'return undef' to "return" is correct because
return returns undef in the scalar context. "return undef" should only
be used when evaluated in the array context and the value undef is
needed instead of ().
I only looked at the case for detect_utf16. If the change is ok for the
other returns, you still have to investigate if you don't trust the
change.
Regards,
Michael
