You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
FontReader class has a local variable stream, which is disposed in the constructor. I believe it's not correct and FontReader class shouldn't dispose it at all, instead FontReader class should implement IDisposable to dispose created MemoryStream.
Small code snippet of proposed changes:
private readonly bool disposable = false;
...
else if (version == 0x774F4632)
{
....
disposable = true;
this.stream = decompressedStream;
}
public void Dispose()
{
if (disposable)
this.stream.Dispose();
}
BigEndianBinaryReader right now has an empty Dispose method, because of this the using statement is not used when created an instance. To be more consistent it'll be good to wrap all instances in using statement. What do you think?
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
@JimBobSquarePants , hi, I've used
IDisposableAnalyzers
to detect some issues. There are some interesting issues which should be fixed.FontReader
class has a local variablestream
, which is disposed in the constructor. I believe it's not correct andFontReader
class shouldn't dispose it at all, instead FontReader class should implement IDisposable to dispose createdMemoryStream
.Small code snippet of proposed changes:
BigEndianBinaryReader
right now has an emptyDispose
method, because of this theusing
statement is not used when created an instance. To be more consistent it'll be good to wrap all instances in using statement. What do you think?I've created a PR #411
Beta Was this translation helpful? Give feedback.
All reactions