rsync bug in clean_flist() while removing duplicted names

[Thomas Osterried]

hello,

i use rsync as a major backend for our automatic debian GNU/linux
workstation synchronisation project at the Free University of Berlin /
Computer Science.

my concept is, that rsync gets the class, group and machine-specific
root-trees as argument, and it's up to him to find and automaticaly
remove duplicated files on the basis of the priority (which is actually
the argument list).

but i had side-effects, for e.g. that some files appeared from a tree
with "lower priority". so i looked what's going, and found that my
/testing.txt (containing it's origin) was transfered multible times: to
be more exact, it was ever second matching tree [the longest file survived].

rsync -vvvv told me a bit more concrete what happened: 
removing duplicate name %s from file list %d 

clean_flist() in flist.c announces that he will remove the duplicated
entry of the _previous_ list. but the file is beeing removed from the
current list, while retaining the file from the previous list.
thus, when the next file list is checked against this one (now "previous"),
the file did not occur in the list anymore (but in the previous-previous),
and it will be kept.
this leads to the bug that only on every second list the file will be
removed.
i found this bug being present in rsync 2.3.2 up to version 2.5.2.


btw, it's quite important for my priority list, that on duplicated files
the latest list wins. if this behaviour of rsync would change in future,
i'd run into deep problems..


the next problem i run into while testing my fixes on my debian woody
-where i compiled rsync 2.3.2 from the package pool with debuild- was,
that the strlcat() function in lib/compat.c was used.
from senfile.c strlcat() was called:
  strlcat(fname,f_name(file),MAXPATHLEN);
but f_name(file) returned (char *) 0. now strlcat() did a strlen on a
nullpointer, and was not very happy.

now, everything works fine. but it may be worth to check if it's ok that
this null-file that sendfile() has in his list exists.

i'll append both fixes here.

btw, rsync is a nice, powerful tool. thanks for your efforts..

regards,

	- thomas
-------------- next part --------------
diff -acr rsync-2.3.2/flist.c rsync-2.3.2-patched/flist.c
*** rsync-2.3.2/flist.c	Mon Feb 18 16:55:30 2002
--- rsync-2.3.2-patched/flist.c	Mon Feb 18 18:26:12 2002
***************
*** 976,982 ****
  			if (verbose > 1 && !am_server)
  				rprintf(FINFO,"removing duplicate name %s from file list %d\n",
  					f_name(flist->files[i-1]),i-1);
! 			free_file(flist->files[i]);
  		} 
  	}
  
--- 976,982 ----
  			if (verbose > 1 && !am_server)
  				rprintf(FINFO,"removing duplicate name %s from file list %d\n",
  					f_name(flist->files[i-1]),i-1);
! 			free_file(flist->files[i-1]);
  		} 
  	}
  
diff -acr rsync-2.3.2/lib/compat.c rsync-2.3.2-patched/lib/compat.c
*** rsync-2.3.2/lib/compat.c	Mon Nov  8 14:15:04 1999
--- rsync-2.3.2-patched/lib/compat.c	Mon Feb 18 18:26:18 2002
***************
*** 131,139 ****
     be one more than the maximum resulting string length */
   size_t strlcat(char *d, const char *s, size_t bufsize)
  {
! 	size_t len1 = strlen(d);
! 	size_t len2 = strlen(s);
! 	size_t ret = len1 + len2;
  
  	if (len1+len2 >= bufsize) {
  		len2 = bufsize - (len1+1);
--- 131,149 ----
     be one more than the maximum resulting string length */
   size_t strlcat(char *d, const char *s, size_t bufsize)
  {
! 	size_t len1;
! 	size_t len2;
! 	size_t ret;
! 
! 	// bugfix
! 	if (!d)
! 		return 0;
! 	len1 = strlen(d);
! 	len2 = (s) ? strlen(s) : 0;
! 	ret = len1 + len2;
! 
! 	if (!s || ret == len1)
! 		return ret;
  
  	if (len1+len2 >= bufsize) {
  		len2 = bufsize - (len1+1);