NativeMemoryStream should not throw ArgumentOutOfRangeException even "Position + count" > Capacity

[zhuxb711]

NativeMemoryStream should not throw ArgumentOutOfRangeException, that is conflict with the design on int Stream.Read(byte[] buffer, int offset, int count)

Vanara/Core/InteropServices/NativeMemoryStream.cs

Line 172 in e6cff0b

if (Position + count > Capacity) throw new ArgumentOutOfRangeException(nameof(count));

For int Stream.Read(byte[] buffer, int offset, int count) if we do not have enough data available just return how many bytes we set in the buffer rather than throw an exception. That is because the parameter count is just a request

For example, if a Stream with length 1000 and current position is 1000, you should not throw exception even the reader set the count to 10000+, but return 0 to let the reader know that they have already reach the end.

[zhuxb711]

Due to this issue, NativeMemoryStream behave different from MemoryStream

[tajbender]

Sounds plausible 👍

[dahall]

When I read the documentation it does show an exception is thrown on this condition.

[tajbender]

👅 Sounds more plausible

The total number of bytes read into the buffer. This can be less than the number of bytes requested if that many bytes are not currently available, or zero (0) if count is 0 or the end of the stream has been reached.

[dahall]

I'm looking at the source for MemoryStream.Read and it throws the same exception for the same condition as NativeMemoryStream. @zhuxb711: Are you seeing something different when actually using MemoryStream?

[zhuxb711]

I'm looking at the source for MemoryStream.Read and it throws the same exception for the same condition as NativeMemoryStream. @zhuxb711: Are you seeing something different when actually using MemoryStream?

Yes, consider the following code:

var buffer = new byte[1000];
var temp1 = new MemoryStream();
var temp2 = new NativeMemoryStream();

temp1.Write(new byte[1000]);
temp2.Write(new byte[1000]);

temp1.Read(buffer, 0, 100); // Normal for MemoryStream, return value is 0
temp2.Read(buffer, 0, 100); // NativeMemoryStream throw ArgumentOutOfRangeException here

I would expect NativeMemoryStream behave exactly same as MemoryStream

[tajbender]

Hmm, without testing this out... But the streams are both uninitialized, aren't they? Both point to the same array of uninitialized values.

And I don't get the point, why you want to alter Vanara for all its users with their already well tested software, which would all of them require to test things completely out, again?

For a bug that's, well, your fault anyways, isn't it?

Reading out of bounds is always a bad idea... Just my two cents.

[zhuxb711]

Hmm, without testing this out... But the streams are both uninitialized, aren't they? Both point to the same array of uninitialized values.

And I don't get the point, why you want to alter Vanara for all its users with their already well tested software, which would all of them require to test things completely out, again?

For a bug that's, well, your fault anyways, isn't it?

Reading out of bounds is always a bad idea... Just my two cents.

？？？How could you said that is "Well tested" so it not a bug but my fault? I just point it out it looks like a bug. If you don't want anyone point it out, I could just close this issue and make you happy.

And "Reading out of bounds"... Who tell you that I'm reading out of bound. It's a kind of legal usage, as I said, "count" is just a request not a "bound". Anyone could pass any positive number as they like without care how long the Stream is.

Why you focus on uninitialized array? It's not related to the issue. Pass an initialized array will fix that? Off course not. So It's not related. By the way, if you do not init the array, in C#, it will be init with default value (for byte, default value is 0) (which is different from C++)

[zhuxb711]

Let me explain why I would create this issue. Microsoft's official Image control read the Stream in that way. For example: Stream length is 1000, current position is 500. Image control will still read the Stream by passing 4096 as "count".

As I know, it's a legal usage. For Stream.Read() you should return how much data you actually filled in the buffer that caller give you. Parameter "count" just the maximum limitation. So no need to check whether there still enough data available for the "count". Let caller pass any positive value or even zero. All you need is return the actual data you set in the buffer. Caller will slice the buffer according to your return value.

(Return zero if "count" is positive and the Stream reach the end.)

[zhuxb711]

When I read the documentation it does show an exception is thrown on this condition.

the document said that only throw ArugmentOutOfRangeException when offset or count is negative. However, NativeMemoryStream will check whether still have enough capacity available.

[tajbender]

Never touch a running system, that's what I've been told since about thirty years.

Why would anyone ever change an existing API, a Framework used in Thousands of projects, in a critical manner, because you are too lazy to properly catch exceptions?

That's just my humble opinion.

As I know, it's a legal usage. For Stream.Read() you should return how much data you filled in the buffer that caller give you. Parameter "count" just the maximum limitation. So no need to check whether there still enough data available for the "count". Let caller pass any positive value or even zero. All you need is return the actual data you set in the buffer.

Even this may be the case in your Usage, it is not in mine. As said another day, just overwrite the class if it doesn't fit your means. Or just catch the exception, if you really need to.

BTW, did you ever test the native function in your scenario? Perhaps it's just a bug in your usage code, Runtime error, or just an issue known since windows 3.11, but for exact these reasons, never has been touched?

Even an RTC issue may be possible, or a bug in that particular .net version you are using.

Sorry, I don't know what we are talking about here, but I'm out.

if (Position + count > Capacity) throw new ArgumentOutOfRangeException(nameof(count));

If you ever used assembler or c/c++, you know that this statement is exactly what the machine does.

try { this.myOwnNativeStream.Readalot(); } catch(Exception e) { dOh(e); }

Try this, happy coding 😮

[dahall]

Thank you both for your comments. I think I see how to make a simple adjustment and satisfy most of the concerns while lining up the behavior in the sample provided by @zhuxb711. I'll get that done today and note here for your review.

[dahall]

Please review changes in commit referenced above. We were looking at the wrong line. The problem was a few lines of code down.