Fix a bug in file_util::Delete() where symlinks are not handled right

Fix a bug in file_util::Delete() where symlinks are not handled right

With this patch, file_util::Delete() uses lstat64() instead of stat64(), so that Delete() checks the existence of the symbolic link itself not a file pointed by the symlink. This allows us to delete a symbolic link when it points to a non-existent file. 

BUG=119430
TEST=base_unittests::FileUtilTest.* passes.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=145060

[satorux1, 2012-06-29 18:25:20]

http://codereview.chromium.org/10690047/diff/3001/base/file_util_posix.cc
File base/file_util_posix.cc (right):

http://codereview.chromium.org/10690047/diff/3001/base/file_util_posix.cc#new...
base/file_util_posix.cc:210: int test_link = CallLstat(path_str, &file_info);
test_link -> test. lstat() checks regular files too.

http://codereview.chromium.org/10690047/diff/3001/base/file_util_unittest.cc
File base/file_util_unittest.cc (right):

http://codereview.chromium.org/10690047/diff/3001/base/file_util_unittest.cc#...
base/file_util_unittest.cc:748: TEST_F(FileUtilTest,
DeleteSymlinkOfExistentFile) {
DeleteSymlinkToExistentFile

http://codereview.chromium.org/10690047/diff/3001/base/file_util_unittest.cc#...
base/file_util_unittest.cc:767: TEST_F(FileUtilTest,
DeleteSymlinkOfNonExistentFile) {
DeleteSymlinkToNonExistentFile

http://codereview.chromium.org/10690047/diff/3001/base/file_util_unittest.cc#...
base/file_util_unittest.cc:767: TEST_F(FileUtilTest,
DeleteSymlinkOfNonExistentFile) {
DeleteSymlinkToNonExistentFile

http://codereview.chromium.org/10690047/diff/3001/base/file_util_unittest.cc#...
base/file_util_unittest.cc:771:
EXPECT_TRUE(file_util::PathExists(non_existent));
You don't need a dummy file to create a dangling symlink

http://codereview.chromium.org/10690047/diff/3001/base/file_util_unittest.cc#...
base/file_util_unittest.cc:783: EXPECT_FALSE(file_util::PathExists(file_link));
I'd suggest to remove this part too.

[satorux1, 2012-06-29 18:26:13]

BTW, i'm not a base owner. please add brettw or willchan once you address my
comments.

[yoshiki, 2012-06-29 19:12:27]

brettw: Could you take a look and approve to check-in?

http://codereview.chromium.org/10690047/diff/3001/base/file_util_posix.cc
File base/file_util_posix.cc (right):

http://codereview.chromium.org/10690047/diff/3001/base/file_util_posix.cc#new...
base/file_util_posix.cc:210: int test_link = CallLstat(path_str, &file_info);
On 2012/06/29 18:25:20, satorux1 wrote:
> test_link -> test. lstat() checks regular files too.

Done.

http://codereview.chromium.org/10690047/diff/3001/base/file_util_unittest.cc
File base/file_util_unittest.cc (right):

http://codereview.chromium.org/10690047/diff/3001/base/file_util_unittest.cc#...
base/file_util_unittest.cc:748: TEST_F(FileUtilTest,
DeleteSymlinkOfExistentFile) {
On 2012/06/29 18:25:20, satorux1 wrote:
> DeleteSymlinkToExistentFile

Done.

http://codereview.chromium.org/10690047/diff/3001/base/file_util_unittest.cc#...
base/file_util_unittest.cc:767: TEST_F(FileUtilTest,
DeleteSymlinkOfNonExistentFile) {
On 2012/06/29 18:25:20, satorux1 wrote:
> DeleteSymlinkToNonExistentFile

Done.

http://codereview.chromium.org/10690047/diff/3001/base/file_util_unittest.cc#...
base/file_util_unittest.cc:771:
EXPECT_TRUE(file_util::PathExists(non_existent));
On 2012/06/29 18:25:20, satorux1 wrote:
> You don't need a dummy file to create a dangling symlink

Done.

http://codereview.chromium.org/10690047/diff/3001/base/file_util_unittest.cc#...
base/file_util_unittest.cc:783: EXPECT_FALSE(file_util::PathExists(file_link));
On 2012/06/29 18:25:20, satorux1 wrote:
> I'd suggest to remove this part too.

Done.

[brettw, 2012-06-29 19:15:33]

Can you add a comment to Delete in file_util.h mentioning how symbolic links are
handled on posix?

LGTM with that (I didn't look at the unittest).

[yoshiki, 2012-06-30 04:09:08]

brettw: Thanks, I added the comment. This patch will be in the CQ shortly.

[commit-bot: I haz the power, 2012-06-30 04:09:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoshiki@chromium.org/10690047/12001

[commit-bot: I haz the power, 2012-06-30 05:27:01]

Change committed as 145060