Created attachment 14205 [details] Patch to handle ENOTDIR in flist.c:send_directory() If rsync is called with doubled --fuzzy flag and --link-dest (or --compare-dest, --copy-dest), it fails in a situation as described below. The problem appears in both version 3.1.3 and the current development version 3.1.4dev. Let there be the following directory structure $PWD/ `- src/ `- a/ `- f `- ref/ `- a where src/a/f and ref/a are regular files and the files src/a and ref/a actually have the same name. To create it: $ mkdir -p src/a ref; touch src/a/f ref/a Now let rsync copy files from src/ to tgt/ linking to (fuzzy matched) files in ref/, as follows: $ rsync --recursive --fuzzy --fuzzy --link-dest=../ref src/ tgt Rsync fails with: rsync: opendir "/tmp/rsync-test/tgt/../ref/a" failed: Not a directory (20) rsync error: some files/attrs were not transferred (see previous errors) (code 23) at main.c(1189) [sender=3.1.3] This is due to rsync trying to find a fuzzy match for src/a/f in ref/a, which obviously fails, as ref/a is not a directory. The problem arises in flist.c:send_directory() when called by flist.c:get_dirlist() which in turn is called by generator.c:recv_generator(), where the former two are called with ../ref/a . Since the case of an inexistant directory is already handled in flist.c:send_directory(), that is probably also where to handle the case of the filename referring to a file not being a directory. See the attached patch. In fact, I am not sure that it is a good idea to just ignore ENOENT and ENOTDIR if !am_sender, since those should also possibly mean a real error. Or am I mistaken here?
Maybe more or less related: Bug 11866 [0], Bug 12489 [1] [0] https://bugzilla.samba.org/show_bug.cgi?id=11866 [1] https://bugzilla.samba.org/show_bug.cgi?id=12489
Nice catch, I was able to easily reproduce this issue just creating a directory with the name of a just-deleted file. The path you mention Einhard seems to be the only one where no check is done to be sure a directory is given to send_directory(). I think it would however be better to explicitly tells send_directory() that fbuf could not be a directory. Perhaps creating a new FLAG_PERHAPS_NOTDIR flag ? recv_generator() would add FLAG_PERHAPS_NOTDIR to GDL_IGNORE_FILTER_RULES in its get_dirlist() call, get_dirlist() would then send this flag to send_directory(), and send_directory() would properly return if fbuf is not a directory and (flags & FLAG_PERHAPS_NOTDIR).
We could also stat() fnamecmpbuf in recv_generator(), but I think it's rather interesting to save such calls.
Did I understand correctly that you were able to reproduce this in a notably different way? I had not sufficiently examined the code to see that in all the other cases the existance of a directory is made sure before. At least under this precondition, I like your idea to add such a flag. However, wouldn't it then be consequent to also use flags in order to say when a file may not exist at all? Again, I have not thoroughly read the whole code, however I presume there are cases (also, if !am_sender) where the existance of a file (being a directory) is assumed and a violation of this assumption causes trouble and hence should not be ignored.
I reproduced the issue the same way, I meant just creating a directory in my backed-up tree with the name of a just-deleted file, this file remaining in the link-dest folder. I'm not sure the opposite (an existing directory used as if it were a file) exists, as several tests (stating, opening) must be performed by rsync on files before using them. Though I did not go through the whole code path looking for such a case.
Created attachment 14231 [details] Patch using FLAG_PERHAPS_DIR Here is a working patch using the method detailed in comment #2.
Wayne, let's merge this ? Many thanks !
Thanks for the patch! I've committed a slightly tweaked version to git.
Good news, thank you very much Wayne ! Glad to see you back :)