Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StreamRead appears to have misconfigured parameters #693

Open
negentropicdev opened this issue Aug 19, 2021 · 1 comment
Open

StreamRead appears to have misconfigured parameters #693

negentropicdev opened this issue Aug 19, 2021 · 1 comment

Comments

@negentropicdev
Copy link

negentropicdev commented Aug 19, 2021

VIREO_FUNCTION_SIGNATURE4(StreamRead, FileHandle, TypedArrayCoreRef, Int32, Int32)
{
FileHandle handle = _Param(0);
TypedArrayCoreRef array = _Param(1);
Int32 numElts = _Param(2);
Int32 bytesToRead = 0;
if (numElts == -1) {
struct stat fileInfo;
fstat(handle, &fileInfo);
bytesToRead = (Int32) fileInfo.st_size;
// TODO(fileio) is rounding correct here?
// Only read full elements from the file
numElts = bytesToRead / array->ElementType()->TopAQSize();
}
if (array->Resize1DOrEmpty(numElts)) {
// Final count is determined by how big the array ended up.
bytesToRead = array->AQBlockLength(array->Length());
#ifdef VIREO_POSIX_FILEIO
ssize_t bytesRead = POSIX_NAME(read)(handle, array->RawBegin(), bytesToRead);
#else
#error platform not supported
#endif
if (bytesRead < 0) {
_Param(3) = (Int32) bytesRead; // TODO(fileio) error processing
array->Resize1D(0);
} else if (bytesToRead != bytesRead) {
// size the array to the number of full elements read.
array->Resize1D((IntIndex) (bytesRead / array->ElementType()->TopAQSize()) );
}
}
return _NextInstruction();
}

StreamRead has parameters: FileHandle, Output String, Num Elements, Return Status (Based on my interpretation of their usage in the method), however on line 438 NumElements is configured as an output.

I'd like to propose to swap parameters 2 and 3 and change the prototype definition to:
p(i(FileHandle)i(Int32)o(String)o(Int32))

As it's currently:
p(i(FileHandle)o(String)o(Int32)o(Int32))

I didn't see any tests referring to StreamRead so I presume it's likely not in use.

@negentropicdev
Copy link
Author

With this arrangement we're able to leverage StreamRead for input in our PicoG project:
StdIn

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant