Re: possible typo/bug in receiver.c

2003-01-20 Thread Dave Dykstra 
On Sat, Jan 18, 2003 at 12:25:05AM -0800, Wayne Davison wrote:
> On Fri, Jan 17, 2003 at 11:39:31PM -0800, Craig Barratt wrote:
> > If mkstemp() fails (for various reasons, including the directory not
> > existing) then fd == -1.  So the first if () executes, which flushes
> > the data and does a continue.  So the next two if () statements will
> > never execute.
> 
> Good catch.  The code got tweaked into this form in version 1.32 when it
> was changed from a set of calls (using do_mktemp() and do_open()) into a
> single call (using just do_mkstemp()).  At that point the new (simpler)
> error-check section got added, and it made it impossible to get down to
> the older section.
> 
> > Is rsync meant to create deep directories that don't exist?
> 
> Not usually.  In -R mode, it sends implied directories for the files, so
> it should be the case that the directories are normally created already.
> I find it interesting that this old code exists because there has been a
> lot of talk about the sending of implied directories (and an option to
> turn this off).  If we put this code back the way it was back in 2001,
> then the newly suggested --no-implied-dirs option would not cause rsync
> to fail if a deep file didn't have a directory yet -- rsync would just
> create the intervening directories.  I'm not sure if I want it to do
> that or not at the moment.
> 
> ...wayne..


I removed the error check that skipped the creation of the directory,
just a piece of the code that was added in version 1.32.  It's pretty
clear to me that it was just an oversight, because Tridge also modified
the dead code leg.

- Dave
-- 
To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.tuxedo.org/~esr/faqs/smart-questions.html

Re: possible typo/bug in receiver.c

2003-01-18 Thread Wayne Davison 
On Fri, Jan 17, 2003 at 11:39:31PM -0800, Craig Barratt wrote:
> If mkstemp() fails (for various reasons, including the directory not
> existing) then fd == -1.  So the first if () executes, which flushes
> the data and does a continue.  So the next two if () statements will
> never execute.

Good catch.  The code got tweaked into this form in version 1.32 when it
was changed from a set of calls (using do_mktemp() and do_open()) into a
single call (using just do_mkstemp()).  At that point the new (simpler)
error-check section got added, and it made it impossible to get down to
the older section.

> Is rsync meant to create deep directories that don't exist?

Not usually.  In -R mode, it sends implied directories for the files, so
it should be the case that the directories are normally created already.
I find it interesting that this old code exists because there has been a
lot of talk about the sending of implied directories (and an option to
turn this off).  If we put this code back the way it was back in 2001,
then the newly suggested --no-implied-dirs option would not cause rsync
to fail if a deep file didn't have a directory yet -- rsync would just
create the intervening directories.  I'm not sure if I want it to do
that or not at the moment.

..wayne..
-- 
To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.tuxedo.org/~esr/faqs/smart-questions.html

possible typo/bug in receiver.c

2003-01-17 Thread Craig Barratt 
The following code in receiver.c around line 421 (2.5.6pre1) contains
some dead code:

/* we initially set the perms without the
   setuid/setgid bits to ensure that there is no race
   condition. They are then correctly updated after
   the lchown. Thanks to [EMAIL PROTECTED] for pointing
   this out.  We also set it initially without group
   access because of a similar race condition. */
fd2 = do_mkstemp(fnametmp, file->mode & INITACCESSPERMS);
if (fd2 == -1) {
rprintf(FERROR,"mkstemp %s failed: %s\n",fnametmp,strerror(errno));
receive_data(f_in,buf,-1,NULL,file->length);
if (buf) unmap_file(buf);
if (fd1 != -1) close(fd1);
continue;
}

/* in most cases parent directories will already exist
   because their information should have been previously
   transferred, but that may not be the case with -R */
if (fd2 == -1 && relative_paths && errno == ENOENT &&
create_directory_path(fnametmp, orig_umask) == 0) {
strlcpy(fnametmp, template, sizeof(fnametmp));
fd2 = do_mkstemp(fnametmp, file->mode & INITACCESSPERMS);
}
if (fd2 == -1) {
rprintf(FERROR,"cannot create %s : %s\n",fnametmp,strerror(errno));
receive_data(f_in,buf,-1,NULL,file->length);
if (buf) unmap_file(buf);
if (fd1 != -1) close(fd1);
continue;
}

If mkstemp() fails (for various reasons, including the directory not
existing) then fd == -1.  So the first if () executes, which flushes
the data and does a continue.  So the next two if () statements will
never execute.

It might be an editing error (not sure how old it is).  It looks
like the first if () statement was meant to be replaced by the
next two; ie: the first if () statement should be eliminated.

I haven't backed out a command-level example that shows the difference,
but it relates to receiving into a path whose last two or more
directories don't exist.  Is rsync meant to create deep directories
that don't exist?

Craig
-- 
To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.tuxedo.org/~esr/faqs/smart-questions.html