Some additions to support symlinks better on platforms that support them.

Some additions to support symlinks better on platforms that support them.

BUG=none
TEST=Ran new unit test, passed trybots.

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

[Greg Spencer (Chromium), 2010-11-29 20:52:37]

I developed this a while ago while working on some ChromeOS code that was
manipulating symlinks.  I didn't check it in at the time because there was too
much going on, but I think it's better to be using this than sprinkling
symlink/readlink calls in the code, so we can keep with using FilePaths, etc.

[Evan Martin, 2010-11-29 21:21:14]

In general, we end up with a copy of this code in every Chrome-derived binary;
for example, our PDF plugin gets a copy of this.  So I am opposed to adding
utility code that doesn't have an immediate use.

It sounds like you have an immediate use in ChromeOS, so this change LGTM.  But
I would probably say delete the Windows-related TODO since it doesn't need an
implementation unless for some reason we need to write Vista-specific symlink
code.

http://codereview.chromium.org/5349007/diff/1/base/file_util_posix.cc
File base/file_util_posix.cc (right):

http://codereview.chromium.org/5349007/diff/1/base/file_util_posix.cc#newcode390
base/file_util_posix.cc:390: if (count > 0) {
Consider an early return pattern here instead (if (count <= 0) return false;)

http://codereview.chromium.org/5349007/diff/1/base/file_util_posix.cc#newcode391
base/file_util_posix.cc:391: *target_path = FilePath(FilePath::StringType(buf,
count));
Can use std::string here, since we know the string type.

[Greg Spencer (Chromium), 2010-11-29 21:34:11]

Yes, it's my intention to start using this in the ChromeOS code once it's
available.  (This was a separable change, so I separated it.)

http://codereview.chromium.org/5349007/diff/1/base/file_util_posix.cc
File base/file_util_posix.cc (right):

http://codereview.chromium.org/5349007/diff/1/base/file_util_posix.cc#newcode390
base/file_util_posix.cc:390: if (count > 0) {
On 2010/11/29 21:21:14, Evan Martin wrote:
> Consider an early return pattern here instead (if (count <= 0) return false;)

Done.

http://codereview.chromium.org/5349007/diff/1/base/file_util_posix.cc#newcode391
base/file_util_posix.cc:391: *target_path = FilePath(FilePath::StringType(buf,
count));
On 2010/11/29 21:21:14, Evan Martin wrote:
> Can use std::string here, since we know the string type.

I prefer to use the actual type that FilePath expects, actually.  I'm aware that
StringType is just a typedef for std::string on posix, but if that ever changed,
or this code were ported to other platforms, using the StringType here makes it
a lot easier to find instances that are supposed to be interpreted as FilePaths
as opposed to strings used for other purposes (i.e. it's more self documenting).

For this particular instance, this isn't a huge deal, but as a coding practice,
I think it is.