A simple solution that would get us pretty far would be to offer an IO module that was a higher-level wrapper around file-descriptor-based open/read/write/close OS level APIs.
Created attachment 225008 [details] Patch
Comment on attachment 225008 [details] Patch Actually, this patch still has issues.
Created attachment 225261 [details] Patch
Comment on attachment 225261 [details] Patch 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!
Comment on attachment 225261 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=225261&action=review > Tools/jsc-cli/jsc-cli/lib/IOModule.m:150 > + int result = fclose(handle.stream); > + if (result != 0) can we clear the stream field here? I don't want any bogus reuse to occur
(In reply to comment #4) > (From update of attachment 225261 [details]) > 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. It's relatively slow due to lots of copying across the API boundary. I'm not sure we could avoid this without new API functionality. > > > 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 I'm not sure what you mean by this. Why should you get the same error if you don't have permission to access the file as when the file doesn't exist? That seems like a pure usability regression.
(In reply to comment #5) > (From update of attachment 225261 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=225261&action=review > > > Tools/jsc-cli/jsc-cli/lib/IOModule.m:150 > > + int result = fclose(handle.stream); > > + if (result != 0) > > can we clear the stream field here? I don't want any bogus reuse to occur Will do.
> > Tools/jsc-cli/jsc-cli/lib/IOModule.m:145 > > + return [[NSString alloc] initWithBytes:line length:length encoding:NSUTF8StringEncoding]; > > Will this copy the bytes? Yep. - initWithBytesNoCopy:length:encoding: is the version that doesn't copy the bytes.