Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughNetworkFetcher의 공개 API가 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Source/AppleFetcher/Implement/NetworkFetcher.swift (1)
70-81: 즉시 실행 클로저(IIFE) 패턴 단순화 제안현재 로직은 정확하지만, 즉시 실행 클로저 대신 computed property 스타일이나 별도 함수로 추출하면 가독성이 향상될 수 있습니다.
♻️ 선택적 리팩토링 제안
let family = interface.ifa_addr.pointee.sa_family - guard let protocolVersion = { - switch Int32(family) { - case AF_INET: - return InterfaceType.ProtocolVersion.v4 - case AF_INET6: - return InterfaceType.ProtocolVersion.v6 - default: - return nil - } - }() - else { continue } + let protocolVersion: InterfaceType.ProtocolVersion + switch Int32(family) { + case AF_INET: + protocolVersion = .v4 + case AF_INET6: + protocolVersion = .v6 + default: + continue + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Source/AppleFetcher/Implement/NetworkFetcher.swift` around lines 70 - 81, The inline immediately-invoked closure computing protocolVersion from interface.ifa_addr.pointee.sa_family is unnecessary; replace it with a small helper (e.g., a static/function like protocolVersion(forFamily: Int32) -> InterfaceType.ProtocolVersion?) or a computed property that switches on the family and returns .v4, .v6 or nil, then call that helper to produce protocolVersion and guard-let it (referring to interface.ifa_addr.pointee.sa_family, family, and InterfaceType.ProtocolVersion in the replacement).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Source/AppleFetcher/Implement/NetworkFetcher.swift`:
- Around line 106-113: Rename the misspelled variable searchs to searches in
NetworkFetcher.swift and update all references to it (e.g., any later uses
around where InterfaceType list is consumed) so the new identifier is
consistent; ensure variable declaration at InterfaceType(...) array and every
subsequent access (such as the usage mentioned near the later block) are updated
to the new name to avoid compile errors.
---
Nitpick comments:
In `@Source/AppleFetcher/Implement/NetworkFetcher.swift`:
- Around line 70-81: The inline immediately-invoked closure computing
protocolVersion from interface.ifa_addr.pointee.sa_family is unnecessary;
replace it with a small helper (e.g., a static/function like
protocolVersion(forFamily: Int32) -> InterfaceType.ProtocolVersion?) or a
computed property that switches on the family and returns .v4, .v6 or nil, then
call that helper to produce protocolVersion and guard-let it (referring to
interface.ifa_addr.pointee.sa_family, family, and InterfaceType.ProtocolVersion
in the replacement).
This change flattens the IP struct by removing the InterfaceType wrapper and directly embedding interface name and protocol version as properties. This simplifies the data structure and reduces nesting while maintaining the same functionality. fix: correct typo in variable name 'searchs' to 'searches'
minjae999
force-pushed
the
feature/fetch_ip_list
branch
from
d3babc4 to
cb065e3
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Source/AppleFetcher/Implement/NetworkFetcher.swift`:
- Around line 53-55: ipStorage currently uses the interface name as the sole key
so IPv4 and IPv6 entries overwrite each other; change the storage to include the
IP version (for example make ipStorage a dictionary keyed by (interfaceName,
IP.Version) or a nested [String: [IP.Version: String]]) and update the code
paths that write/read ipStorage (the code using the ipStorage variable and the
loop that iterates over searches) so each interface can hold both v4 and v6
addresses without clobbering; keep interfacesPointer usage unchanged but ensure
insertion uses the new key shape and any lookups (later code that reads
ipStorage) are updated to account for the versioned key.
- Around line 65-76: The code dereferences interface.ifa_addr directly which can
be nil and crash; in the protocolVersion determination (the closure using
interface.ifa_addr.pointee.sa_family) add a guard let addr = interface.ifa_addr
to early-skip when nil, then use addr.pointee.sa_family (reusing the
non-optional pointer) to map AF_INET/AF_INET6 to IP.Version.v4/v6; apply the
same nil-check and pointer reuse to the similar block around lines 78-89 to
avoid crashes when ifa_addr is nil.
- Simplify ipStorage structure to use IP objects directly instead of tuples - Rename interfaceType to interfaceName for clarity - Streamline searches array by removing redundant version specifications - Simplify filtering logic by removing unnecessary map operations
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Source/AppleFetcher/Implement/NetworkFetcher.swift`:
- Around line 65-76: The code dereferences interface.ifa_addr without nil-check
causing possible crashes; update the logic in the guard computing
protocolVersion (and the subsequent getnameinfo call) to safely optional-bind
interface.ifa_addr (e.g., guard let addrPtr = interface.ifa_addr else { continue
}), use addrPtr.pointee.sa_family when switching to determine IP.Version (in
protocolVersion) and unwrap the sockaddr pointer before passing to getnameinfo
so both the family/switch and the getnameinfo invocation skip interfaces with
nil ifa_addr rather than force-dereferencing.
- Line 53: ipStorage currently uses only the interface name as the key (var
ipStorage: [String: IP]) so IPv4 and IPv6 for the same interface overwrite each
other; update the storage to distinguish address families by either using a
composite key (e.g., interface + family) or change ipStorage to a nested
structure or array (e.g., [String: [IPFamily: IP]] or [String: [IP]]) and update
any code that writes/reads ipStorage in NetworkFetcher.swift (references:
ipStorage variable and the assignment logic around the same file, including the
code near line 93) to store and retrieve both v4 and v6 entries without
clobbering.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Source/AppleFetcher/Implement/NetworkFetcher.swift (1)
53-108:⚠️ Potential issue | 🔴 Critical
freeifaddrs이중 호출로 double free 가능성이 있습니다.Line 56의
defer로 해제한 뒤 Line 106에서 다시 호출되어 크래시 위험이 있습니다. 하나만 유지하세요.🛠️ 수정 제안
- freeifaddrs(interfacesPointer) - return ipList🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Source/AppleFetcher/Implement/NetworkFetcher.swift` around lines 53 - 108, The code calls freeifaddrs twice causing a potential double-free: remove the explicit freeifaddrs(interfacesPointer) at the end and rely on the existing defer { freeifaddrs(interfacesPointer) } that is declared next to interfacesPointer; ensure interfacesPointer is still declared as var interfacesPointer: UnsafeMutablePointer<ifaddrs>? and that getifaddrs(&interfacesPointer) and the sequence loop (using interfaceFirstPointer) remain unchanged so cleanup happens exactly once.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@Source/AppleFetcher/Implement/NetworkFetcher.swift`:
- Around line 53-108: The code calls freeifaddrs twice causing a potential
double-free: remove the explicit freeifaddrs(interfacesPointer) at the end and
rely on the existing defer { freeifaddrs(interfacesPointer) } that is declared
next to interfacesPointer; ensure interfacesPointer is still declared as var
interfacesPointer: UnsafeMutablePointer<ifaddrs>? and that
getifaddrs(&interfacesPointer) and the sequence loop (using
interfaceFirstPointer) remain unchanged so cleanup happens exactly once.
- Fix whitespace in guard statement - Remove unnecessary freeifaddrs call
The defer statement was being executed even if getifaddrs failed, potentially calling freeifaddrs on an uninitialized pointer.
0xWOF
approved these changes
Feb 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.