[webkit-reviews] review granted: [Bug 237477] Add null check for path in makeAllDirectories : [Attachment 453859] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Sihui Liu <sihui_liu at apple.com>'s
request for review:
Bug 237477: Add null check for path in makeAllDirectories
https://bugs.webkit.org/show_bug.cgi?id=237477

Attachment 453859: Patch

https://bugs.webkit.org/attachment.cgi?id=453859&action=review




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 453859
  --> https://bugs.webkit.org/attachment.cgi?id=453859
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=453859&action=review

> Source/WTF/wtf/posix/FileSystemPOSIX.cpp:280
> +	   LOG_ERROR("File failed to access. Error message: %s",
safeStrerror(errno).data());

I think "file failed to access" is a vague message to put in a log, and not how
I would describe a failure of the access function. If we are going to log this,
then the message should make some mention of the fact that were were attempting
to create directories, perhaps. Also wondering why we decided to log about the
failure of this access call but not about the failures of the other access and
mkdir calls below.

Generally we want these file system abstraction functions like
makeAllDirectories to not log directly to the console, so callers can make
calls they know will or might fail without unconditionally cluttering logs. I
understand that this conflicts with the desire to know what’s happening when
diagnosing a bug. I’m not sure there is an easy answer. Generally speaking it
would not be good if all file system calls logged what happened directly to the
console, and certainly that’s not how the underlying POSIX functions behave.