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

delta_diff_generator.cc

 // Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "update_engine/delta_diff_generator.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #include <inttypes.h>
 #include <sys/stat.h>
@@ skipping 151 matching lines @@
     LOG(INFO) << "Encoding file " << fs_iter.GetPartialPath();
 
     // We can't visit each dst image inode more than once, as that would
     // duplicate work. Here, we avoid visiting each source image inode
     // more than once. Technically, we could have multiple operations
     // that read the same blocks from the source image for diffing, but
     // we choose not to to avoid complexity. Eventually we will move away
     // from using a graph/cycle detection/etc to generate diffs, and at that
     // time, it will be easy (non-complex) to have many operations read
     // from the same source blocks. At that time, this code can die. -adlr
-    bool should_diff_from_source = true;
+    bool should_diff_from_source = false;
     string src_path = old_root + fs_iter.GetPartialPath();
-    if (utils::FileExists(src_path.c_str())) {
-      struct stat src_stbuf;
-      TEST_AND_RETURN_FALSE_ERRNO(0 == stat(src_path.c_str(), &src_stbuf));
+    struct stat src_stbuf;

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

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...

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

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).

+    if (0 == lstat(src_path.c_str(), &src_stbuf) &&

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

There's a utils::IsSymlink utility... You could add a IsRegularFile or
something...

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

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.

+        S_ISREG(src_stbuf.st_mode)) {
       should_diff_from_source = !utils::SetContainsKey(visited_src_inodes,
                                                        src_stbuf.st_ino);
       visited_src_inodes.insert(src_stbuf.st_ino);
     }
 
     TEST_AND_RETURN_FALSE(DeltaReadFile(graph,
                                         Vertex::kInvalidIndex,
                                         blocks,
                                         (should_diff_from_source ?
                                          old_root :
@@ skipping 1439 matching lines @@
   dummy_extent->set_start_block(kSparseHole);
   dummy_extent->set_num_blocks((signature_blob_length + kBlockSize - 1) /
                                kBlockSize);
 }
 
 const char* const kBsdiffPath = "bsdiff";
 const char* const kBspatchPath = "bspatch";
 const char* const kDeltaMagic = "CrAU";
 
 };  // namespace chromeos_update_engine