Re: Find file dialog in mc-4.6.0-pre3

[Arpi <arpi mplayerhq hu>]

Hi,

> I've scanned a TODO list and found the item -
> "Make find dialog more responsive" - inside
> "After 4.6.1 on 4.6.x branch" section.
> 
> But I think the responsiveness of the dialog is unacceptable for the
> 4.6.0 release. I missed the moment when it became so bad, (may be 1-3

I agree.

So i went and hunted down the bug :)
It's a nice 100l one, said in MPlayer terminology...

In src/key.c, the is_idle() function is broken.
It's called by the dialog loop (used by Find File too) to decide if
call the callback fv (search for files) or handle the key/mouse events.

The original code ends this way:

    timeout.tv_sec = 0;
    timeout.tv_usec = 0;
    select (maxfdp, &select_set, 0, 0, &timeout);
    return !FD_ISSET (0, &select_set);

which is, according to 'man select' broken at 2 points:

       int  select(int  n,  fd_set  *readfds,  fd_set  *writefds,
       fd_set *exceptfds, struct timeval *timeout);

       n is the highest-numbered descriptor in any of  the  three
       sets, plus 1.
             ^^^^^^
so it should be:

    select (maxfdp+1, &select_set, 0, 0, &timeout);

this may work on some select() implementations, i've heard that some ignores
the first parameter and calculates it from the fd sets. but it's better fixed.

and:

       FD_ISSET(int fd, fd_set *set);

       FD_ISSET  tests  to  see  if  a
       descriptor is part of the set; this is useful after select
       returns.

so it should be:

    return !FD_ISSET (input_fd, &select_set);

(the original code works only if input_fd==0)

after changing these, find is responsive again!

btw, NOTE: i didn't handled the gpm events, they need extra code,
so the current is_idle() should be changed deep. I've just summarized
the idea about the bug. If you want, i can prepare a commitable patch.


A'rpi / Astral & ESP-team

--
Developer of MPlayer, the Movie Player for Linux - http://www.MPlayerHQ.hu