mbeijen / File-MimeInfo

Perl module for determining file types using the freedesktop.org shared mime-info database
https://metacpan.org/module/File::MimeInfo
20 stars 13 forks source link

Warnings when no handler found #49

Closed Smylers closed 2 years ago

Smylers commented 2 years ago

Invoking mime_applications on a mime type for which no applications are found yields a bunch of warnings to the terminal:

$ perl -MFile::MimeInfo::Applications  -wE "mime_applications q[application/octet-stream]"
Use of uninitialized value in subroutine entry at /usr/share/perl5/File/BaseDir.pm line 117.
Use of uninitialized value in subroutine entry at /usr/share/perl5/File/BaseDir.pm line 117.
Use of uninitialized value in subroutine entry at /usr/share/perl5/File/BaseDir.pm line 117.
Use of uninitialized value in subroutine entry at /usr/share/perl5/File/BaseDir.pm line 117.
Use of uninitialized value in subroutine entry at /usr/share/perl5/File/BaseDir.pm line 117.
Use of uninitialized value in subroutine entry at /usr/share/perl5/File/BaseDir.pm line 117.

(If you have a handler for application/octed-stream, I think just making up a Mime type would demonstrate the issue.)

I think the cause is in _find_file(). _default() invokes it like this:

my @list = _read_list(@paths);
my $desktop_file = _find_file(reverse @list);

If no handlers are specified, _read_list() returns an empty list, so inside _find_file(), @_ is empty. But the start of _find_file() is:

sub _find_file {
    my @list = shift;
    for (@list) {

shift returns a scalar value, so when @_ is empty, @list gets set to (undef). The for loop then iterates over that undef, and warnings ensure.

That for loop is also odd when @_ does contain multiple values: the first one gets shifted into @list and is processed by the loop — but either way the loop runs precisely once, making the looping part unnecessary.

I'm wondering if it should be:

sub _find_file {
    my @list = @_;
    for (@list) {

That would avoid looping at all when @_ is empty, and would also mean that when it isn't empty would loop over all its items, not just the first.

If it's desirable only to ‘loop’ over the first then maybe putting return undef if !@_ at the very top of the sub would do that and avoid the warnings.

Thanks.

mbeijen commented 2 years ago

Hi, thanks for the report! And you're right....

yesterday I merged 8519aee96271a17c2a626625ccaa2f1e29f26a76 which does exactly as you proposed, and released it to cpan as version 0.33.