From df34965cd7e6a333503d24f098061a66906dea65 Mon Sep 17 00:00:00 2001 From: George Barnett Date: Fri, 3 Nov 2023 10:04:31 +0000 Subject: [PATCH] Don't reserve capacity for NIOPCAPRingBuffer (#209) Motivation: The NIOPCAPRingBuffer can limit the number of fragment or the total number of bytes in its buffer or both. When configuring the buffer to limit only the maximum number of bytes it sets the maximum number of fragments allowed to `.max`. On `init` the buffer has enough capacity reserved to store tha maximum number of fragments. This would be a large and potentially totally unnecessary allocation. That is, if it didn't crash at runtime. It crashes at runtime as `CircularBuffer` converts the requested capacity to a `UInt32` which traps if you pass it an `Int.max`. Modifications: - Don't reserve capacity on init - Adjust the test which tests the byte limit to not set a capacity as well Result: - `NIOPCAPRingBuffer(maximumBytes:)` doesn't crash --- Sources/NIOExtras/PCAPRingBuffer.swift | 5 +++-- Tests/NIOExtrasTests/PCAPRingBufferTest.swift | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Sources/NIOExtras/PCAPRingBuffer.swift b/Sources/NIOExtras/PCAPRingBuffer.swift index 0199556..dabcd77 100644 --- a/Sources/NIOExtras/PCAPRingBuffer.swift +++ b/Sources/NIOExtras/PCAPRingBuffer.swift @@ -35,7 +35,8 @@ public class NIOPCAPRingBuffer { self.maximumFragments = maximumFragments self.maximumBytes = maximumBytes self.pcapCurrentBytes = 0 - self.pcapFragments = CircularBuffer(initialCapacity: maximumFragments) + // Don't default to `maximumFragments` as it will be `.max` on some paths. + self.pcapFragments = CircularBuffer() } /// Initialise the buffer, setting constraints @@ -88,7 +89,7 @@ public class NIOPCAPRingBuffer { } /// Emit the captured data to a consuming function; then clear the captured data. - /// - Returns: A ciruclar buffer of captured fragments. + /// - Returns: A circular buffer of captured fragments. public func emitPCAP() -> CircularBuffer { let toReturn = self.pcapFragments // Copy before clearing. self.pcapFragments.removeAll(keepingCapacity: true) diff --git a/Tests/NIOExtrasTests/PCAPRingBufferTest.swift b/Tests/NIOExtrasTests/PCAPRingBufferTest.swift index 4034325..5f3fed3 100644 --- a/Tests/NIOExtrasTests/PCAPRingBufferTest.swift +++ b/Tests/NIOExtrasTests/PCAPRingBufferTest.swift @@ -66,7 +66,7 @@ class PCAPRingBufferTest: XCTestCase { func testByteLimit() { let expectedData = 150 + 25 + 75 + 120 - let ringBuffer = NIOPCAPRingBuffer(maximumFragments: 1000, maximumBytes: expectedData + 10) + let ringBuffer = NIOPCAPRingBuffer(maximumBytes: expectedData + 10) for fragment in dataForTests() { ringBuffer.addFragment(fragment) }