On 06/04/2015 11:35 PM, Amit Kapila wrote:
On Fri, Jun 5, 2015 at 7:29 AM, Andrew Dunstan <and...@dunslane.net
<mailto:and...@dunslane.net>> wrote:
On 06/04/2015 09:23 AM, Amit Kapila wrote:
Okay, as we both seem to agree that it can be mostly used in
tablespace symlinks context, so I have changed the name to
remove_tablespace_symlink() and moved the function to
tablespace.c. S_ISLINK check is used for non-windows code,
so not sure adding it here makes any real difference now that
we have made it specific to tablespace and we might need to
write small port specific code if we want to add S_ISLINK
check.
Where is it used? I can't see it called at all in tablespace.c or
xlog.c.
Below files use S_ISLINK check
basebackup.c, fd.c, initdb.c, copy_fetch.c, pg_rewind/filemap.c
and all these places use it with #ifndef WIN32
Perhaps I'm being overcautious, but here's more or less what I had
in mind.
What is making you feel nervous, if it is that we should not
use unlink call without checking S_ISLINK, then we are
already doing the same at many other places (rewriteheap.c,
slru.c, timeline.c, xlog.c). It is already defined for Windows
as pgunlink.
Theoretically, I don't see much problem by changing the checks
way you have done in patch, but it becomes different than what
we have in destroy_tablespace_directories() and it is slightly
changing the way check was originally done in
create_tablespace_directories(), basically original check will try
unlink if lstat returns non-zero return code. If you want to proceed
with the changed checks as in v3, then may be we can modify
comments on top of function remove_tablespace_symlink() which
indicates that it works like destroy_tablespace_directories().
The difference is that here we're getting the list from a base backup
and it seems to me the risk of having a file we don't really want to
unlink is significantly greater.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
