The dangers of static buffers in rsync code

[Cristian Gafton]

I have been trying for quite a while now to understand why is the 
flist.c:f_name() function implemented using static buffers. Anyone care to 
comment?

The immediate problem is that any call to f_name overrides the previous
content (well, obvious). This, combined with the fact that several
function calls are made with the result of f_name(file) results in
problems handling hardlinks - and possibly other issues as well, but
hardlinks is what made me look deeper inside the rsync code.

As a side note, the f_name function has some "protection" against this
static clobbering: instead of having a single buffer, it has... TEN, and
it cycles through them. That's both funny and sad...

One obvious problem path is starting from generator.c:generate_files()  
function. In there we call in a while loop:

recv_generator(local_name?local_name:f_name(file),flist,i,f), which calls
  hlink.c:check_hard_link(), which calls
    hlink.c:hlink_compare(), which calls
      flist.c:file_compare(), which calls
        flist.c:f_name().

Interested parties might want to follow the code, it is easier and more
obvious...

Anyway, the end result is that in recv_generator() the first argument 
(fname) is now clobbered by our search through the hlink_list, and 
depending on what was the last thing we put in there there are various 
failures happening.

The bad part is that they are mostly silent. Most of the bitching is done
via "if (verbose > 1)", so with the standard 'rsync -av' there are no
indicators that would tell somebody rsync failed to handle a particular
file...

One way of "unclobbering" this is by forcing another call to f_name after
the check_hard_link() call is finished (wither in check_hard_link itself
or in recv_generator), but that smells rather nasty... On the other hand,
reimplementing f_name to make it sane would require some bigger changes,
since f_name() happens to be quite a popular function inside the rsync
code...

This is bad and needs some fixing... Little hammer or big hammer?

Cristian
--
----------------------------------------------------------------------
Cristian Gafton     --     gafton at redhat.com      --     Red Hat, Inc.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  "There are two kinds of people who never amount to much: those who 
   cannot do what they are told, and those who can do nothing else."
        --Cyrus Curtis