[webkit-reviews] review granted: [Bug 132346] For DARWIN platforms, use system temporary directory for DataLog output : [Attachment 230395] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Michael Saboff
<msaboff at apple.com>'s request for review:
Bug 132346: For DARWIN platforms, use system temporary directory for DataLog
output
https://bugs.webkit.org/show_bug.cgi?id=132346

Attachment 230395: Patch
https://bugs.webkit.org/attachment.cgi?id=230395&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=230395&action=review


> Source/WTF/ChangeLog:8
> +	   Added code to call confstr() to access the process' temporary
directory to use that

"process's" -- there's only one.

> Source/WTF/wtf/DataLog.cpp:84
> +    size_t lastComponentLength = strlen(logBasename) + 10;
> +    size_t dirnameLength = confstr(_CS_DARWIN_USER_TEMP_DIR, filenameBuffer,
1024);
> +    if ((dirnameLength + lastComponentLength + 10) < maxPathLength) {
> +	   strncat(filenameBuffer, logBasename, maxPathLength - dirnameLength);

> +	   filename = filenameBuffer;
> +    }

Why + 10?

> Source/WTF/wtf/DataLog.cpp:106
>	   if (!file)
> -	       fprintf(stderr, "Warning: Could not open log file %s for
writing.\n", actualFilename);
> +	       WTFLogAlways("Warning: Could not open log file %s for
writing.\n", actualFilename);
> +#if DATA_LOG_TO_DARWIN_TEMP_DIR
> +	   else
> +	       WTFLogAlways("*** Logging to \"%s\" ***\n", actualFilename);
> +#endif

Better to say "if (file) / else" rather than "if (!file) / else".