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

io.stream_reader read function does not work #21768

Open
thomas-mangin opened this issue Jun 30, 2024 · 5 comments
Open

io.stream_reader read function does not work #21768

thomas-mangin opened this issue Jun 30, 2024 · 5 comments
Labels
Bug This tag is applied to issues which reports bugs.

Comments

@thomas-mangin
Copy link
Contributor

thomas-mangin commented Jun 30, 2024

Describe the bug

I am attempting to use an io.stream_reader.StringReader to parse an $embed_file and pass the result to a function accepting an io.Reader interface. There is no test in test suite to ensure the io.Reader interface is working as should.

This is failing because the implementation of StringReader.new has an issue and does not initialise the io.reader within its structure when it is not provided, and source is used instead.

It may not be obvious as when read_all is called, it will use read_all_bytes, which then calls r.fill_buffer(read_till_end_of_stream) or {} hidding the issue.

fill_buffer will have failed as mut reader := r.reader or { return error('reader is not set') } will have returned an error, but the code will still work as there is data to consume.

With the io.reader interface read() calls the folling code is called read := r.fill_buffer_until(buf.len - start)!, which has the same guard for .reader and bubbles up the error.

It seems that the or {} is wrong and hides the fact that the .reader was not initialised, but as the buffer exists, it is "working".

Reproduction Steps

module main

import io.string_reader

fn main() {
	s := 'line 1\nline 2\n'

	// works
	mut r := string_reader.StringReader.new(source: s)
	assert r.read_all(false)! == s

	// fails
	mut b := []u8{len: 100}
	mut sr := string_reader.StringReader.new(source: s)
	sr.read(mut b)!
}

Expected Behavior

StringReader can be used when initialised with a source as an io.Reader

Current Behavior

V panic: result not set (reader is not set)

Possible Solution

Ensure there is a reader created associated with the underlying data or change the guard.

Additional Information/Context

No response

V version

V 0.4.6 df7828d

Environment details (OS name and version, etc.)

V full version: V 0.4.6 cc14272.df7828d
OS: macos, macOS, 14.5, 23F79
Processor: 10 cpus, 64bit, little endian, Apple M1 Max

getwd: /Users/thomas/Code/github.com/ze-community/ze/main
vexe: /Users/thomas/Unix/local/v/master/v
vexe mtime: 2024-06-30 12:18:18

vroot: OK, value: /Users/thomas/Unix/local/v/master
VMODULES: OK, value: /Users/thomas/Unix/data/v/modules
VTMP: OK, value: /tmp/v_501

Git version: git version 2.45.2
Git vroot status: weekly.2024.26-18-gdf7828d1-dirty
.git/config present: true

CC version: Apple clang version 15.0.0 (clang-1500.3.9.4)
thirdparty/tcc status: thirdparty-macos-arm64 5c1d002f

Note

You can use the 👍 reaction to increase the issue's priority for developers.

Please note that only the 👍 reaction to the issue itself counts as a vote.
Other reactions and those to comments will not be taken into account.

@thomas-mangin thomas-mangin added the Bug This tag is applied to issues which reports bugs. label Jun 30, 2024
@thomas-mangin
Copy link
Contributor Author

@Casper64 I am not sure if you can investigate or not.

@Casper64
Copy link
Member

The read method should only be used if a io.reader instance is passed when creating a new StringReader.

That's confusing, it should be possible to use the read method seperately. 👍

@Casper64
Copy link
Member

Casper64 commented Jun 30, 2024

Sorry, to come back at this:

In your example you pass in a buffer of lenght 100. If you call read the stringreader will attempt to read 100 bytes. The given source is only 4 bytes long, so then the code tries to read from the io.reader contained in it, but that is not set, there is no data and the method returns an error.

To me this makes sense, but the error message doesn't.

I think in this case the method should return the io.Eof error. What do you think @thomas-mangin ?

@thomas-mangin
Copy link
Contributor Author

Hi @Casper64, and thank you for answering.

You are right: I messed up my minimum viable example; my buffer length should have been smaller than the string length.

In the case above, if only 14 bytes were read, the function should return 14 and the next call should return Eof. I believe this is the expected behaviour of the function in the C language and what people would expect.

As the above buffer is 100, then either another system call to the OS is required to read more than 14 bytes (for networking, the data may still be in transit) or only 14 bytes are available. The next call would then return Eof.

The problem with the reader not getting set is still there with a small buffer size as you can see below where the buffer length is 4.

module main

import io.string_reader

fn main() {
	s := 'line 1\nline 2\n'

	// works
	mut r := string_reader.StringReader.new(source: s)
	assert r.read_all(false)! == s

	// fails
	mut b := []u8{len: 4}
	mut sr := string_reader.StringReader.new(source: s)
	sr.read(mut b)!
}
❯ v run bug.v  
V panic: result not set (reader is not set)
v hash: df7828d
0   bug                                 0x0000000104a76838 panic_result_not_set + 124
1   bug                                 0x0000000104a8d650 main__main + 1000
2   bug                                 0x0000000104a8d95c main + 84
3   dyld                                0x000000018ebe60e0 start + 2360

@Casper64
Copy link
Member

Casper64 commented Jul 1, 2024

That will be fixed. When the length of the buffer is smaller or equal to the length of the provided source the string reader will read from that source always, regardless of whether a reader is set or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This tag is applied to issues which reports bugs.
Projects
None yet
Development

No branches or pull requests

2 participants