Skip to content

Commit 953b5de

Browse files
lilithlilith
committed
Fix various code warnings, ObjectDisposedHelper, Culture Invariance, Exception usage
1 parent a211198 commit 953b5de

14 files changed

+45
-64
lines changed

‎.editorconfig

+2-2
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,8 @@ charset = utf-8-bom
9494

9595
[*.{cs,vb}]
9696

97-
# SYSLIB1054: Use 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time
98-
dotnet_diagnostic.SYSLIB1054.severity = warning
97+
# # SYSLIB1054: Use 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time
98+
# dotnet_diagnostic.SYSLIB1054.severity = warning
9999

100100
# CA1018: Mark attributes with AttributeUsageAttribute
101101
dotnet_diagnostic.CA1018.severity = warning

‎src/Imageflow/Bindings/ImageflowException.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ internal static ImageflowException FromContext(JobContextHandle c, ulong default
6565
return new ImageflowException("Imageflow context has no error stored; cannot fetch error message");
6666
case ErrorFetchResult.BufferTooSmall: break;
6767
default:
68-
throw new ArgumentOutOfRangeException();
68+
throw new NotImplementedException($"Unknown error fetching error: {result}");
6969
}
7070
}
7171

‎src/Imageflow/Bindings/JobContext.cs

+6-17
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,7 @@ private JobContextHandle Handle
1919
{
2020
get
2121
{
22-
if (!_handle.IsValid)
23-
{
24-
throw new ObjectDisposedException("Imageflow JobContext");
25-
}
26-
22+
ObjectDisposedHelper.ThrowIf(!_handle.IsValid, this);
2723
return _handle;
2824
}
2925
}
@@ -354,11 +350,7 @@ private unsafe ImageflowJsonResponse InvokeInternal(ReadOnlySpan<byte> nullTermi
354350

355351
public void AssertReady()
356352
{
357-
if (!_handle.IsValid)
358-
{
359-
throw new ObjectDisposedException("Imageflow JobContext");
360-
}
361-
353+
ObjectDisposedHelper.ThrowIf(!_handle.IsValid, this);
362354
if (HasError)
363355
{
364356
throw ImageflowException.FromContext(Handle);
@@ -646,7 +638,7 @@ public void AddOutputBuffer(int ioId)
646638
[Obsolete("Use a higher-level wrapper like the Fluent API instead; they can use faster code paths")]
647639
public Stream GetOutputBuffer(int ioId)
648640
{
649-
if (!_ioSet.ContainsKey(ioId) || _ioSet[ioId] != IoKind.OutputBuffer)
641+
if (!_ioSet.TryGetValue(ioId, out var value) || value != IoKind.OutputBuffer)
650642
{
651643
throw new ArgumentException($"ioId {ioId} does not correspond to an output buffer", nameof(ioId));
652644
}
@@ -669,7 +661,7 @@ public Stream GetOutputBuffer(int ioId)
669661
/// <exception cref="ImageflowAssertionFailed"></exception>
670662
internal unsafe Span<byte> BorrowOutputBuffer(int ioId)
671663
{
672-
if (!_ioSet.ContainsKey(ioId) || _ioSet[ioId] != IoKind.OutputBuffer)
664+
if (!_ioSet.TryGetValue(ioId, out var value) || value != IoKind.OutputBuffer)
673665
{
674666
throw new ArgumentException($"ioId {ioId} does not correspond to an output buffer", nameof(ioId));
675667
}
@@ -693,7 +685,7 @@ internal unsafe Span<byte> BorrowOutputBuffer(int ioId)
693685
/// <exception cref="ImageflowAssertionFailed"></exception>
694686
internal IMemoryOwner<byte> BorrowOutputBufferMemoryAndAddReference(int ioId)
695687
{
696-
if (!_ioSet.ContainsKey(ioId) || _ioSet[ioId] != IoKind.OutputBuffer)
688+
if (!_ioSet.TryGetValue(ioId, out var value) || value != IoKind.OutputBuffer)
697689
{
698690
throw new ArgumentException($"ioId {ioId} does not correspond to an output buffer", nameof(ioId));
699691
}
@@ -721,10 +713,7 @@ internal void RemoveRef()
721713
public bool IsDisposed => !_handle.IsValid;
722714
public void Dispose()
723715
{
724-
if (IsDisposed)
725-
{
726-
throw new ObjectDisposedException("Imageflow JobContext");
727-
}
716+
ObjectDisposedHelper.ThrowIf(IsDisposed, this);
728717

729718
if (Interlocked.Exchange(ref _refCount, 0) > 0)
730719
{

‎src/Imageflow/Bindings/JobContextHandle.cs

+2-5
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ namespace Imageflow.Bindings;
99
/// <summary>
1010
/// The handle is ready even if there is an error condition stored in the context.
1111
///
12-
/// AddRef and Release should be called.
12+
/// AddRef and Release should be called.
1313
/// </summary>
1414
internal sealed class JobContextHandle : SafeHandleZeroOrMinusOneIsInvalid, IAssertReady
1515
{
@@ -39,10 +39,7 @@ public JobContextHandle()
3939

4040
public void AssertReady()
4141
{
42-
if (!IsValid)
43-
{
44-
throw new ObjectDisposedException("Imageflow JobContextHandle");
45-
}
42+
ObjectDisposedHelper.ThrowIf(!IsValid, this);
4643
}
4744

4845
public ImageflowException? DisposeAllowingException()

‎src/Imageflow/Bindings/JsonResponse.cs

+1-4
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,7 @@ internal ImageflowJsonResponse(JsonResponseHandle ptr)
9999
public void AssertReady()
100100
{
101101
_handle.ParentContext.AssertReady();
102-
if (!_handle.IsValid)
103-
{
104-
throw new ObjectDisposedException("Imageflow JsonResponse");
105-
}
102+
ObjectDisposedHelper.ThrowIf(!_handle.IsValid, this);
106103
}
107104

108105
private void Read(out int statusCode, out IntPtr utf8Buffer, out UIntPtr bufferSize)

‎src/Imageflow/Bindings/JsonResponseHandle.cs

+4-11
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ namespace Imageflow.Bindings;
88

99
/// <summary>
1010
/// A child SafeHandle that increments the reference count on JobContextHandle when created and decrements it when disposed.
11-
///
11+
///
1212
/// </summary>
1313
internal sealed class JsonResponseHandle : SafeHandleZeroOrMinusOneIsInvalid, IAssertReady
1414
{
@@ -33,15 +33,8 @@ public JsonResponseHandle(JobContextHandle parent, IntPtr ptr)
3333

3434
public void AssertReady()
3535
{
36-
if (!ParentContext.IsValid)
37-
{
38-
throw new ObjectDisposedException("Imageflow JobContextHandle");
39-
}
40-
41-
if (!IsValid)
42-
{
43-
throw new ObjectDisposedException("Imageflow JsonResponseHandle");
44-
}
36+
ObjectDisposedHelper.ThrowIf(!ParentContext.IsValid, ParentContext);
37+
ObjectDisposedHelper.ThrowIf(!IsValid, this);
4538
}
4639

4740
#pragma warning disable SYSLIB0004
@@ -51,7 +44,7 @@ protected override bool ReleaseHandle()
5144
{
5245
// The base class, the caller, handles interlocked / sync and preventing multiple calls.
5346
// We check ParentContext just in case someone went wild with DangerousRelease elsewhere.
54-
// It's a process-ending error if ParentContext is invalid.
47+
// It's a process-ending error if ParentContext is invalid.
5548
if (ParentContext.IsValid)
5649
{
5750
NativeMethods.imageflow_json_response_destroy(ParentContext, handle);

‎src/Imageflow/Bindings/NativeLibraryLoading.cs

+13-13
Original file line numberDiff line numberDiff line change
@@ -45,34 +45,34 @@ internal void RaiseException()
4545
{
4646
var sb = new StringBuilder(_log.Select((e) => e.Basename?.Length ?? 0 + e.FullPath?.Length ?? 0 + 20)
4747
.Sum());
48-
sb.AppendFormat("Looking for \"{0}\" RID=\"{1}-{2}\", IsUnix={3}, IsDotNetCore={4} RelativeSearchPath=\"{5}\"\n",
48+
sb.AppendFormat(CultureInfo.InvariantCulture, "Looking for \"{0}\" RID=\"{1}-{2}\", IsUnix={3}, IsDotNetCore={4} RelativeSearchPath=\"{5}\"\n",
4949
Filename,
5050
RuntimeFileLocator.PlatformRuntimePrefix.Value,
5151
RuntimeFileLocator.ArchitectureSubdir.Value, RuntimeFileLocator.IsUnix,
5252
RuntimeFileLocator.IsDotNetCore.Value,
5353
AppDomain.CurrentDomain.RelativeSearchPath);
5454
if (FirstException != null)
5555
{
56-
sb.AppendFormat("Before searching: {0}\n", FirstException.Message);
56+
sb.AppendFormat(CultureInfo.InvariantCulture, "Before searching: {0}\n", FirstException.Message);
5757
}
5858

5959
foreach (var e in _log)
6060
{
6161
if (e.PreviouslyLoaded)
6262
{
63-
sb.AppendFormat("\"{0}\" is already {1}", e.Basename, Verb);
63+
sb.AppendFormat(CultureInfo.InvariantCulture, "\"{0}\" is already {1}", e.Basename, Verb);
6464
}
6565
else if (!e.FileExists)
6666
{
67-
sb.AppendFormat("File not found: {0}", e.FullPath);
67+
sb.AppendFormat(CultureInfo.InvariantCulture, "File not found: {0}", e.FullPath);
6868
}
6969
else if (e.LoadErrorCode.HasValue)
7070
{
7171
string errorCode = e.LoadErrorCode.Value < 0
7272
? string.Format(CultureInfo.InvariantCulture, "0x{0:X8}", e.LoadErrorCode.Value)
7373
: e.LoadErrorCode.Value.ToString(CultureInfo.InvariantCulture);
7474

75-
sb.AppendFormat("Error \"{0}\" ({1}) loading {2} from {3}",
75+
sb.AppendFormat(CultureInfo.InvariantCulture, "Error \"{0}\" ({1}) loading {2} from {3}",
7676
new Win32Exception(e.LoadErrorCode.Value).Message,
7777
errorCode,
7878
e.Basename, e.FullPath);
@@ -83,7 +83,7 @@ internal void RaiseException()
8383
var installed = Environment.Is64BitProcess ? "32-bit (x86)" : "64-bit (x86_64)";
8484
var needed = Environment.Is64BitProcess ? "64-bit (x86_64)" : "32-bit (x86)";
8585

86-
sb.AppendFormat("\n> You have installed a {0} copy of imageflow.dll but need the {1} version",
86+
sb.AppendFormat(CultureInfo.InvariantCulture, "\n> You have installed a {0} copy of imageflow.dll but need the {1} version",
8787
installed, needed);
8888
}
8989

@@ -93,12 +93,12 @@ internal void RaiseException()
9393
var crtLink = "https://aka.ms/vs/16/release/vc_redist."
9494
+ (Environment.Is64BitProcess ? "x64.exe" : "x86.exe");
9595

96-
sb.AppendFormat("\n> You may need to install the C Runtime from {0}", crtLink);
96+
sb.AppendFormat(CultureInfo.InvariantCulture, "\n> You may need to install the C Runtime from {0}", crtLink);
9797
}
9898
}
9999
else
100100
{
101-
sb.AppendFormat("{0} {1} in {2}", Verb, e.Basename, e.FullPath);
101+
sb.AppendFormat(CultureInfo.InvariantCulture, "{0} {1} in {2}", Verb, e.Basename, e.FullPath);
102102
}
103103
sb.Append('\n');
104104
}
@@ -262,7 +262,7 @@ private static IEnumerable<Tuple<bool, string>> BaseFolders(IEnumerable<string>?
262262
try{
263263
// Look in the folder that *this* assembly is located.
264264
assemblyLocation = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location);
265-
265+
266266
} catch (NotImplementedException){
267267
// ignored
268268
}
@@ -352,7 +352,7 @@ private static string GetFilenameWithoutDirectory(string basename) => RuntimeFil
352352

353353
private static readonly Lazy<ConcurrentDictionary<string, string>> ExecutablePathsByName = new Lazy<ConcurrentDictionary<string, string>>(() => new ConcurrentDictionary<string, string>(StringComparer.OrdinalIgnoreCase), LazyThreadSafetyMode.PublicationOnly);
354354

355-
// Not yet implemented.
355+
// Not yet implemented.
356356
// static readonly Lazy<ConcurrentDictionary<string, IntPtr>> LibraryHandlesByFullPath = new Lazy<ConcurrentDictionary<string, IntPtr>>(() => new ConcurrentDictionary<string, IntPtr>(StringComparer.OrdinalIgnoreCase), LazyThreadSafetyMode.PublicationOnly);
357357

358358
/// <summary>
@@ -429,7 +429,7 @@ internal static class NativeLibraryLoader
429429
caughtException = b;
430430
}
431431

432-
//Try loading
432+
//Try loading
433433
var logger = new LoadLogger
434434
{ FirstException = caughtException, Filename = GetFilenameWithoutDirectory(basename) };
435435
if (TryLoadByBasename(basename, logger, out _, customSearchDirectories))
@@ -449,7 +449,7 @@ internal static class NativeLibraryLoader
449449

450450
private static readonly Lazy<ConcurrentDictionary<string, IntPtr>> LibraryHandlesByBasename = new Lazy<ConcurrentDictionary<string, IntPtr>>(() => new ConcurrentDictionary<string, IntPtr>(StringComparer.OrdinalIgnoreCase), LazyThreadSafetyMode.PublicationOnly);
451451

452-
// Not yet implemented.
452+
// Not yet implemented.
453453
// static readonly Lazy<ConcurrentDictionary<string, IntPtr>> LibraryHandlesByFullPath = new Lazy<ConcurrentDictionary<string, IntPtr>>(() => new ConcurrentDictionary<string, IntPtr>(StringComparer.OrdinalIgnoreCase), LazyThreadSafetyMode.PublicationOnly);
454454

455455
/// <summary>
@@ -536,7 +536,7 @@ internal static class WindowsLoadLibrary
536536

537537
public static IntPtr Execute(string fileName)
538538
{
539-
// Look in the library dir instead of the process dir
539+
// Look in the library dir instead of the process dir
540540
const uint loadWithAlteredSearchPath = 0x00000008;
541541
return LoadLibraryEx(fileName, IntPtr.Zero, loadWithAlteredSearchPath);
542542
}

‎src/Imageflow/Fluent/IOutputDestinationExtensions.cs

+2
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,9 @@ internal static async Task CopyFromStreamAsyncInternal(this IOutputDestination d
101101

102102
int bytesRead;
103103
while ((bytesRead =
104+
#pragma warning disable CA1835
104105
await stream.ReadAsync(buffer, 0, buffer.Length, cancellationToken).ConfigureAwait(false)) != 0)
106+
#pragma warning restore CA1835
105107
{
106108
await dest.WriteAsync(new ArraySegment<byte>(buffer, 0, bytesRead), cancellationToken)
107109
.ConfigureAwait(false);

‎src/Imageflow/Fluent/ImageJob.cs

+3-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System.Collections.ObjectModel;
22
using System.Diagnostics;
3+
using System.Globalization;
34
using System.Text;
45
using System.Text.Json;
56
using System.Text.Json.Nodes;
@@ -623,7 +624,7 @@ private static JsonNode ToFramewiseGraph(ICollection<BuildItemBase> uniqueNodes)
623624
var nodes = new JsonObject();
624625
foreach (var n in uniqueNodes)
625626
{
626-
nodes.Add((n.Uid - lowestUid).ToString(), n.NodeData);
627+
nodes.Add((n.Uid - lowestUid).ToString(CultureInfo.InvariantCulture), n.NodeData);
627628
}
628629
// return new
629630
// {
@@ -645,10 +646,7 @@ private static JsonNode ToFramewiseGraph(ICollection<BuildItemBase> uniqueNodes)
645646

646647
private void AssertReady()
647648
{
648-
if (_disposed)
649-
{
650-
throw new ObjectDisposedException("ImageJob");
651-
}
649+
ObjectDisposedHelper.ThrowIf(_disposed, this);
652650
}
653651

654652
public void Dispose()

‎src/Imageflow/Fluent/MagicBytes.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,7 @@ internal enum ImageFormat
484484
break;
485485

486486
default:
487-
throw new ArgumentOutOfRangeException();
487+
throw new NotImplementedException();
488488
}
489489
return null;
490490
}

‎src/Imageflow/Fluent/PerformanceDetails.cs

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System.Globalization;
12
using System.Text;
23
using System.Text.Json.Nodes;
34

@@ -16,7 +17,7 @@ internal PerformanceDetails(JsonNode? perf)
1617
// foreach (var f in perf.frames)
1718
// {
1819
// frames.Add(new PerformanceDetailsFrame(f));
19-
// }
20+
// }
2021
if (obj.TryGetPropertyValue("frames", out var framesValue))
2122
{
2223
foreach (var f in framesValue?.AsArray() ?? [])
@@ -33,7 +34,7 @@ public string GetFirstFrameSummary()
3334
var sb = new StringBuilder();
3435
if (Frames.Count > 1)
3536
{
36-
sb.Append($"First of {Frames.Count} frames: ");
37+
sb.Append(string.Format(CultureInfo.InvariantCulture,"First of {0} frames: ", Frames.Count));
3738
}
3839
else if (Frames.Count == 0)
3940
{
@@ -44,7 +45,7 @@ public string GetFirstFrameSummary()
4445
{
4546
sb.Append(n.Name);
4647
sb.Append('(');
47-
sb.Append((n.WallMicroseconds / 1000.0).ToString("0.####"));
48+
sb.Append((n.WallMicroseconds / 1000.0).ToString( "0.####",CultureInfo.InvariantCulture));
4849
sb.Append("ms) ");
4950
}
5051

‎src/Imageflow/Fluent/SrgbColor.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ private static byte Expand4(uint v, int index)
4949
public static SrgbColor FromHex(string s)
5050
{
5151
s = s.TrimStart('#');
52-
var v = uint.Parse(s, NumberStyles.HexNumber);
52+
var v = uint.Parse(s, NumberStyles.HexNumber, CultureInfo.InvariantCulture);
5353
switch (s.Length)
5454
{
5555
case 3: return RGBA(Expand4(v, 2), Expand4(v, 1), Expand4(v, 0), 0xff);

‎src/Imageflow/Internal.Helpers/ArgumentNullThrowHelperPolyfill.cs

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System.Diagnostics;
12
using System.Diagnostics.CodeAnalysis;
23
using System.Runtime.CompilerServices;
34

@@ -24,7 +25,7 @@ public static void ThrowIfNull(
2425
Throw(paramName);
2526
}
2627
#else
27-
Argument.ThrowIfNull(argument, paramName);
28+
ArgumentNullException.ThrowIfNull(argument, paramName);
2829
#endif
2930
}
3031

@@ -39,6 +40,7 @@ internal static void Throw(string? paramName) =>
3940

4041
internal static class ObjectDisposedHelper
4142
{
43+
[StackTraceHidden]
4244
public static void ThrowIf([DoesNotReturnIf(true)] bool condition, object instance)
4345
{
4446
#if NET8_0_OR_GREATER

‎src/Imageflow/Internal.Helpers/StreamMemoryExtensionsPolyfills.cs

+2
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@ static async Task WriteAsyncFallback(Stream stream, ReadOnlyMemory<byte> buffer,
2929
{
3030
buffer.Span.CopyTo(rent);
3131

32+
#pragma warning disable CA1835
3233
await stream.WriteAsync(rent, 0, buffer.Length, cancellationToken).ConfigureAwait(false);
34+
#pragma warning restore CA1835
3335
}
3436
finally
3537
{

0 commit comments

Comments
 (0)