[tz] zic fails for soft-links-only target volume

Tom Lane tgl at sss.pgh.pa.us
Thu Nov 3 14:55:54 UTC 2016


I wrote:
> It appears to me that the problem is that commit 35484e98c broke
> the case of linking to an existing symlink.  This happened because
> itsdir() was modified to return 2 not 0 for a symlink, but dolink()
> throws an EPERM error for that, because it wasn't taught about the
> change.  There's no reason to disallow that case, so I propose this
> patch:

On closer inspection, I notice that there really aren't any callers
of itsdir() that are interested in both conditions simultaneously.
So a less error-prone solution would be to revert 35484e98c's changes
to itsdir(), taking it back to doing only what the name suggests,
and invent an itssymlink() function that has a similar API but tests
for symlink-ness not directory-ness.  Then you'd do this:

-	  staysymlink = itsdir(tofield) == 2;
+	  staysymlink = itssymlink(tofield) > 0;

and leave dolink()'s first usage of itsdir() alone.

I also note that 35484e98c has possibly broken the other caller of
itsdir():

		if (mkdir(name, MKDIR_UMASK) != 0) {
			int err = errno;
			if (err != EEXIST && itsdir(name) < 0) {
				error(_("%s: Can't create directory %s: %s"),
				      progname, name, strerror(err));
				exit(EXIT_FAILURE);
			}
		}

as I doubt we want that to consider "it's a symlink" as an OK outcome.
I would actually say this should be

-			if (err != EEXIST && itsdir(name) < 0) {
+			if (err != EEXIST || itsdir(name) <= 0) {

as we really don't want the code to keep going unless there is a directory
there.  Maybe even drop the test on what the err code is, and only check
itsdir()?

			regards, tom lane


More information about the tz mailing list