[webkit-reviews] review denied: [Bug 129114] jsc-cli needs a basic story for text IO : [Attachment 225261] Patch

[bugzilla-daemon at webkit.org]

Oliver Hunt <oliver at apple.com> has denied Mark Hahnenberg
<mhahnenberg at apple.com>'s request for review:
Bug 129114: jsc-cli needs a basic story for text IO
https://bugs.webkit.org/show_bug.cgi?id=129114

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

------- Additional Comments from Oliver Hunt <oliver at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=225261&action=review


How does this io performance compare to other scripting environments - it
should be as fast or faster.

r- due to buffer overflow

> Tools/jsc-cli/jsc-cli/lib/IOModule.m:102
> +	   char* buffer = (char*)malloc(sizeof(char) * (bytes + 1));
> +	   size_t result = fread(buffer, sizeof(char), bytes, handle.stream);

Hello buffer overflow :)

> Tools/jsc-cli/jsc-cli/lib/IOModule.m:106
> +	       setErrorWithErrno();
> +	       free(buffer);
> +	       return (JSValue *)nil;

Please just throw an error on an io error, don't mess around with separate
errors

> Tools/jsc-cli/jsc-cli/lib/IOModule.m:142
> +	       setErrorWithErrno();

again exception

> Tools/jsc-cli/jsc-cli/lib/IOModule.m:145
> +	   return [[NSString alloc] initWithBytes:line length:length
encoding:NSUTF8StringEncoding];

Will this copy the bytes?

> Tools/jsc-cli/jsc-cli/lib/IOModule.m:151
> +	       setErrorWithErrno();

exception!