[patch] Recursive find file does not search recursively on samba share

[bilbo]

When searching recursively a samba share, mc does not go into any subdirectories even if "Find recursively" is checked.

With a bit of debugging I found a cause:

In src/filemanager/file.c in function do_search, the algorithm executes mc_stat on a file (directory) and checks number of links. It assumes number of links minus 2 is number of subdirectories available for recursive search. While that assumption is valid on probably all linux filesystems, it is not valid on mounted samba shares. For these, number of links (as reported by stat or seen by 'ls -l') is not 1 for most files and 2+ for directories, but zero for all files and directories within the share (I guess that is how the kenrel samba module works ...). Therefore the optimization (that may limit the recursion) done by subdirs_left breaks the recursive search.

This error is in current master branch.

I am attaching a patch that fixes the issue (basically removes the subdirs_left optimization that is broken)

The patch seems not to have any side effects, the search works fine on ext4, vfat and samba share.

[bilbo]

Patch to fix recursive search on samba shares

[andrew_b]

Branch: 3097_smb_recursive_find
changeset:3324ed0727042c5b8b62dc26a3fd72cf2f4e1fb0

[andrew_b]

Merged to master: [3324ed0727042c5b8b62dc26a3fd72cf2f4e1fb0].

[ossi]

ehm ... you fixed it by completely removing the optimization, i.e., pessimized it for everybody because there are some broken systems. interesting approach ...

for samba, it could have been trivially fixed by disabling it only when a directory's initial ref count it zero. i presume it would be the same for the windows-served nfs?

and ... review process? wasn't there something?

[andrew_b]

Replying to ossi:

for samba, it could have been trivially fixed by disabling it only when a directory's initial ref count it zero.

What do you mean? As initially reported,

...number of links minus 2 is number of subdirectories available for recursive search. While that assumption is valid on probably all linux filesystems, it is not valid on mounted samba shares. For these, number of links (as reported by stat or seen by 'ls -l') is not 1 for most files and 2+ for directories, but zero for all files and directories within the share.

[ossi]

hmm? what's so hard about "if it's zero, ignore it"?

[andrew_b]

Replying to ossi:

hmm? what's so hard about "if it's zero, ignore it"?

Please show the your code.

About optimization. Previously, we called mc_lstat() before push_directory() and mc_stat() after pop_directory(). We called stat() twice for the same directory. Now we remove call of mc_stat(). I thing it is some compensation for the removed optimization.

And more. I mount native Windows share via cifs. 'ls -l' shows me 0 links for directory, mc_lstat() too, but mc_stat() gives me st_nlink == 1. As a result, subdirs_left is negative, and subdirs_left-- never comes to zero.

[ossi]

i really don't see the challenge (i haven't looked very closely what obstacles the mc vfs implementation puts into the path, but the algorithm itself has been implemented successfully many times before).

anyway, this discussion is actually pointless, as the optimization is supposed to save stat()ing every found file, but we need to do that anyway.

on a completely unrelated matter, the glib2-based implementation of pop_directory does not match the hand-coded one, and it shows in the erratic directory search order. it should use g_queue_pop_head.