[webkit-reviews] review granted: [Bug 39283] Port SharedBuffer to POSIX. : [Attachment 56471] patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Young Han Lee
<joybro at company100.net>'s request for review:
Bug 39283: Port SharedBuffer to POSIX.
https://bugs.webkit.org/show_bug.cgi?id=39283

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

------- Additional Comments from Darin Adler <darin at apple.com>
> +#include "FileSystem.h"
> +
> +#include <fcntl.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +#include <wtf/text/CString.h>

We put includes in a single paragraph. Not one for "" and a separate one for
<>.

> +    char* filename = filenameFromString(filePath);

Can this function ever fail? If so, then I think we need code to handle that
case.

> +    size_t totalBytesRead = 0;
> +    ssize_t bytesRead;
> +    while ((bytesRead = read(fd, result->m_buffer.data() + totalBytesRead,
bytesToRead - totalBytesRead)) > 0)
> +	   totalBytesRead += bytesRead;

If the length of the file increases due to another process writing to it while
this function is running, then this will end up calling read with a size of 0.
I suppose that's OK and how you're supposed to use the read call.