On 2022-05-01 at 16:33:13 UTC-0400 (Sun, 1 May 2022 16:33:13 -0400)
Kevin A. McGrail <kmcgr...@apache.org>
is rumored to have said:

On 5/1/2022 4:12 PM, Michael Storz wrote:
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.
It's not a matter of trust, it's a matter of changing return undef where there is a comment that says "# let perl figure it out from the BOM".  That comment worries me that the return undef was purposeful.

I don't see how the explicit 'return undef' can have any specific relationship to that comment. The code checks for a UTF16 BOM and if it finds one, does not bother trying to detect whether the data is LE or BE, because with a BOM, Perl (Encode::Detect::Detector) determines that without help when asked to, later in the block that called detect_utf16(). Without a BOM, detect_utf16() is needed to look at the data and try to guess the endianness. With a BOM, detect_utf16() is a no-op and ultimately Encode::Detect::Detector gets used (and should work just fine.)

It helps to understand that detect_utf16() is called in exactly one place: in another part of Node.pm. It is called there in scalar context. In scalar context, 'return' without an argument returns undef. There's no logical reason for it to ever be called in list context, since what it is returning is (a pointer to) an Encode object.

Reply via email to