POSIX: Make sure files in plugin directories are regular files.

POSIX: Make sure files in plugin directories are regular files.

BUG=none
TEST=Visiting about:plugins with MOZ_PLUGIN_PATH=/tmp does not hang when there is a pipe in /tmp.

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

[Evan Martin, 2010-04-25 18:31:04]

http://codereview.chromium.org/1700014/diff/4001/5001
File webkit/glue/plugins/plugin_lib_posix.cc (right):

http://codereview.chromium.org/1700014/diff/4001/5001#newcode48
webkit/glue/plugins/plugin_lib_posix.cc:48: close(fd);
You need to check the return value of this or finicky compilers will complain.

I wonder if a better approach to that is to just use stat() for this test.  The
latter call to file_util::ReadFile will fail if the file is unreadable.  In
other words, currently the code is (open, fstat, close, open, read, close), and
I'm suggesting (stat, open, read, close).

Also, please double check that the S_ISREG test doesn't do the wrong thing on
symlinks.  (I guess it's probably right; it'd only be wrong with lstat...)

[Lei Zhang, 2010-04-26 22:07:03]

http://codereview.chromium.org/1700014/diff/4001/5001
File webkit/glue/plugins/plugin_lib_posix.cc (right):

http://codereview.chromium.org/1700014/diff/4001/5001#newcode48
webkit/glue/plugins/plugin_lib_posix.cc:48: close(fd);
On 2010/04/25 18:31:04, Evan Martin wrote:
> You need to check the return value of this or finicky compilers will complain.

Ah yes, I even sat through a talk that stressed this...

> I wonder if a better approach to that is to just use stat() for this test. 
The
> latter call to file_util::ReadFile will fail if the file is unreadable.  In
> other words, currently the code is (open, fstat, close, open, read, close),
and
> I'm suggesting (stat, open, read, close).

No, doing just stat(pipe, &buf) will hang, that why I opened  it myself with
O_NONBLOCK. I bet stat() is syntactic sugar for {open(), fstat(), close()}
anyway.

> Also, please double check that the S_ISREG test doesn't do the wrong thing on
> symlinks.  (I guess it's probably right; it'd only be wrong with lstat...)

S_ISREG works fine with symlinks.

[Evan Martin, 2010-04-26 22:10:33]

LGTM