AU: When checking if we've visited a file before, don't follow symlinks.

AU: When checking if we've visited a file before, don't follow symlinks.

A bug caused a failure when a file existed on the old image that was a
symlink to a non-existent path. This CL fixes that bug.

BUG=chromium-os:12393
TEST=unittests/generated and tested a delta

Change-Id: Ifba711be2bfc3c1d96a4ed8536b16b5d7b8a0cae

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=48a0a48

[petkov, 2011-02-22 20:04:04]

LGTM w/ a comment below.

http://codereview.chromium.org/6551015/diff/1/delta_diff_generator.cc
File delta_diff_generator.cc (right):

http://codereview.chromium.org/6551015/diff/1/delta_diff_generator.cc#newcode174
delta_diff_generator.cc:174: struct stat src_stbuf;
so, if a symlink in the src image is replaced by a real file in the new image,
we won't diff it? just want to make sure it's intentional...

http://codereview.chromium.org/6551015/diff/1/delta_diff_generator.cc#newcode175
delta_diff_generator.cc:175: if (0 == lstat(src_path.c_str(), &src_stbuf) &&
There's a utils::IsSymlink utility... You could add a IsRegularFile or
something...

[adlr, 2011-02-22 23:21:48]

replied to comments and added a unittest. PTAL.

http://codereview.chromium.org/6551015/diff/1/delta_diff_generator.cc
File delta_diff_generator.cc (right):

http://codereview.chromium.org/6551015/diff/1/delta_diff_generator.cc#newcode174
delta_diff_generator.cc:174: struct stat src_stbuf;
On 2011/02/22 20:04:04, petkov wrote:
> so, if a symlink in the src image is replaced by a real file in the new image,
> we won't diff it? just want to make sure it's intentional...

Yeah, symlinks could point to bunk data (imagine a symlink that points to
somewhere on the host) so we don't want to diff against what a symlink points
to. (We could get into some more advanced checking, but that doesn't seem high
priority).

http://codereview.chromium.org/6551015/diff/1/delta_diff_generator.cc#newcode175
delta_diff_generator.cc:175: if (0 == lstat(src_path.c_str(), &src_stbuf) &&
On 2011/02/22 20:04:04, petkov wrote:
> There's a utils::IsSymlink utility... You could add a IsRegularFile or
> something...

I went to do that, then realized I need to get the inode out, so I still need to
stat it (or make a hybrid isRegularFileAndGetInode function). Figured it was
clean to keep as is.

[petkov, 2011-02-22 23:25:32]

LGTM

http://codereview.chromium.org/6551015/diff/3001/delta_diff_generator.cc
File delta_diff_generator.cc (right):

http://codereview.chromium.org/6551015/diff/3001/delta_diff_generator.cc#newc...
delta_diff_generator.cc:164: // We can't visit each dst image inode more than
once, as that would
Update the comment to reflect the fact that we never diff symlinks...