[AI Task] [Tizen.Network.WiFi] Consolidate GetFound*/GetWiFiConfigurations into EnumerateForeach<T> helper#7680

Open

JoonghyunCho wants to merge 2 commits into

main from

ai-task/issue-7622

Open

[AI Task] [Tizen.Network.WiFi] Consolidate GetFound*/GetWiFiConfigurations into EnumerateForeach<T> helper #7680
JoonghyunCho wants to merge 2 commits into
main from
ai-task/issue-7622

Conversation

@JoonghyunCho

@JoonghyunCho JoonghyunCho commented Jun 7, 2026

Copy link

Copy Markdown

Member

Summary

Consolidates the four near-identical "native foreach callback → List collection" methods in WiFiManagerImpl into a single generic EnumerateForeach<T> helper.

Changes

src/Tizen.Network.WiFi/Tizen.Network.WiFi/WiFiManagerImpl.cs
- Added private generic EnumerateForeach<T>(opName, privilege, nativeForeach, wrap) helper.
- Converted these 4 methods to single-expression bodies delegating to the helper:
  GetFoundAPs, GetFoundSpecificAPs, GetFoundBssids, GetWiFiConfigurations.
- Net change: 48 insertions, 78 deletions.

Behavior preservation

Iteration semantics are identical: the callback adds an item and returns true for a non-zero handle, and returns false otherwise.
CheckReturnValue is still called immediately after the native foreach returns, with the same operation name and the same per-method privilege (PrivilegeNetworkGet for the AP/Bssid getters, PrivilegeNetworkProfile for configurations).
The wrap delegate absorbs the differing clone signatures: Interop.WiFi.AP.Clone(out cloned, original) vs Interop.WiFi.Config.Clone(origin, out cloned).
Public API is unchanged (all four methods are internal).

Mode

Refactoring (behavior-preserving)

Verification

Build: passed (dotnet build src/Tizen.Network.WiFi/Tizen.Network.WiFi.csproj -c Release, 0 errors / 0 warnings)
Tests: N/A (no WiFi unit-test project; internal-only refactor, public API unchanged)
Benchmark: skipped (no sdb device connected — sdb devices reports no targets; manual benchmark verification required)

Fixes #7622


 [Tizen.Network.WiFi] Consolidate GetFound*/GetWiFiConfigurations into...

34b22a3

... EnumerateForeach<T> helper
GetFoundAPs, GetFoundSpecificAPs, GetFoundBssids and
GetWiFiConfigurations each repeated the same 'native foreach callback ->
List collection' pattern, differing only by the native foreach function,
the handle clone call, and the wrapper type.
Introduces a generic EnumerateForeach<T> helper that takes the operation
name, privilege, native foreach function, and an item-wrap delegate. The
wrap delegate absorbs the differing Clone signatures (AP.Clone has the
out handle first; Config.Clone has it second). Iteration semantics,
ret-check timing, exception mapping (CheckReturnValue) and per-method
privilege are preserved, so behavior is unchanged.
Fixes #7622

@JoonghyunCho JoonghyunCho requested a review from chleun-moon as a code owner

June 7, 2026 11:13

@JoonghyunCho JoonghyunCho added the ai-task label

Jun 7, 2026

@github-actions github-actions Bot added the API15 label

Jun 7, 2026

gemini-code-assist[bot]

gemini-code-assist Bot reviewed

Jun 7, 2026

View reviewed changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link

Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the GetFoundAPs, GetFoundSpecificAPs, GetFoundBssids, and GetWiFiConfigurations methods by introducing a shared helper method, EnumerateForeach<T>, to eliminate code duplication when iterating over native handles. The feedback recommends catching and safely rethrowing exceptions within the native callback to prevent undefined behavior from propagating managed exceptions across P/Invoke boundaries. Additionally, the reviewer advises checking the return values of the native clone operations (Interop.WiFi.AP.Clone and Interop.WiFi.Config.Clone) to handle potential cloning failures gracefully.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

src/Tizen.Network.WiFi/Tizen.Network.WiFi/WiFiManagerImpl.cs

Comment on lines +189 to 208

private List<T> EnumerateForeach<T>(string opName, string privilege,

Func<SafeWiFiManagerHandle, Interop.WiFi.HandleCallback, IntPtr, int> nativeForeach,

Func<IntPtr, T> wrap)

{

Log.Info(Globals.LogTag, "GetFoundAPs");

List<WiFiAP> apList = new List<WiFiAP>();

Interop.WiFi.HandleCallback callback = (IntPtr apHandle, IntPtr userData) =>

Log.Info(Globals.LogTag, opName);

List<T> list = new List<T>();

Interop.WiFi.HandleCallback callback = (IntPtr handle, IntPtr userData) =>

{

if (apHandle != IntPtr.Zero)

if (handle != IntPtr.Zero)

{

IntPtr clonedHandle;

Interop.WiFi.AP.Clone(out clonedHandle, apHandle);

WiFiAP apItem = new WiFiAP(clonedHandle);

apList.Add(apItem);

list.Add(wrap(handle));

return true;

}

return false;

};

int ret = Interop.WiFi.GetForeachFoundAPs(GetSafeHandle(), callback, IntPtr.Zero);

CheckReturnValue(ret, "GetForeachFoundAPs", PrivilegeNetworkGet);

return apList;

int ret = nativeForeach(GetSafeHandle(), callback, IntPtr.Zero);

CheckReturnValue(ret, opName, privilege);

return list;

}

@gemini-code-assist gemini-code-assist Bot Jun 7, 2026

Copy link

Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Propagating managed exceptions through native code boundaries (P/Invoke) can lead to undefined behavior, native crashes, or silent failures. Since the wrap delegate can throw exceptions (e.g., if cloning fails or during object construction), it is highly recommended to catch any exceptions inside the callback, stop the iteration, and safely rethrow the captured exception after the native foreach method returns.

 private List<T> EnumerateForeach<T>(string opName, string privilege,
 Func<SafeWiFiManagerHandle, Interop.WiFi.HandleCallback, IntPtr, int> nativeForeach,
 Func<IntPtr, T> wrap)
 {
 Log.Info(Globals.LogTag, opName);
 List<T> list = new List<T>();
 Exception capturedException = null;
 Interop.WiFi.HandleCallback callback = (IntPtr handle, IntPtr userData) =>
 {
 if (handle != IntPtr.Zero)
 {
 try
 {
 list.Add(wrap(handle));
 }
 catch (Exception ex)
 {
 capturedException = ex;
 return false;
 }
 return true;
 }
 return false;
 };
 int ret = nativeForeach(GetSafeHandle(), callback, IntPtr.Zero);
 if (capturedException != null)
 {
 throw capturedException;
 }
 CheckReturnValue(ret, opName, privilege);
 return list;
 }

src/Tizen.Network.WiFi/Tizen.Network.WiFi/WiFiManagerImpl.cs

Comment on lines +213 to +217

apHandle =>

{

IntPtr clonedHandle;

Interop.WiFi.AP.Clone(out clonedHandle, apHandle);

WiFiAP apItem = new WiFiAP(clonedHandle);

apList.Add(apItem);

return true;

}

return false;

};

int ret = Interop.WiFi.GetForeachFoundSpecificAPs(GetSafeHandle(), callback, IntPtr.Zero);

CheckReturnValue(ret, "GetForeachFoundSpecificAPs", PrivilegeNetworkGet);

return apList;

}

internal IEnumerable<WiFiAP> GetFoundBssids()

{

Log.Info(Globals.LogTag, "GetFoundBssids");

List<WiFiAP> apList = new List<WiFiAP>();

Interop.WiFi.HandleCallback callback = (IntPtr apHandle, IntPtr userData) =>

{

if (apHandle != IntPtr.Zero)

Interop.WiFi.AP.Clone(out IntPtr clonedHandle, apHandle);

return new WiFiAP(clonedHandle);

});

@gemini-code-assist gemini-code-assist Bot Jun 7, 2026

Copy link

Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The return value of Interop.WiFi.AP.Clone is ignored. If cloning fails, it may return a non-zero error code and leave clonedHandle as IntPtr.Zero or invalid, which will cause downstream failures when the WiFiAP instance is used. It is safer to check the return value and throw an exception if cloning fails.

 apHandle =>
 {
 int ret = Interop.WiFi.AP.Clone(out IntPtr clonedHandle, apHandle);
 if (ret != (int)WiFiError.None)
 {
 WiFiErrorFactory.ThrowWiFiException(ret, "Failed to clone AP handle");
 }
 return new WiFiAP(clonedHandle);
 });

src/Tizen.Network.WiFi/Tizen.Network.WiFi/WiFiManagerImpl.cs

Comment on lines +222 to +226

apHandle =>

{

IntPtr clonedHandle;

Interop.WiFi.AP.Clone(out clonedHandle, apHandle);

WiFiAP apItem = new WiFiAP(clonedHandle);

apList.Add(apItem);

return true;

}

return false;

};

int ret = Interop.WiFi.GetForeachFoundBssids(GetSafeHandle(), callback, IntPtr.Zero);

CheckReturnValue(ret, "GetForeachFoundBssids", PrivilegeNetworkGet);

return apList;

}

internal IEnumerable<WiFiConfiguration> GetWiFiConfigurations()

{

Log.Debug(Globals.LogTag, "GetWiFiConfigurations");

List<WiFiConfiguration> configList = new List<WiFiConfiguration>();

Interop.WiFi.HandleCallback callback = (IntPtr configHandle, IntPtr userData) =>

{

if (configHandle != IntPtr.Zero)

Interop.WiFi.AP.Clone(out IntPtr clonedHandle, apHandle);

return new WiFiAP(clonedHandle);

});

@gemini-code-assist gemini-code-assist Bot Jun 7, 2026

Copy link

Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

 apHandle =>
 {
 int ret = Interop.WiFi.AP.Clone(out IntPtr clonedHandle, apHandle);
 if (ret != (int)WiFiError.None)
 {
 WiFiErrorFactory.ThrowWiFiException(ret, "Failed to clone AP handle");
 }
 return new WiFiAP(clonedHandle);
 });

src/Tizen.Network.WiFi/Tizen.Network.WiFi/WiFiManagerImpl.cs

Comment on lines +231 to +235

apHandle =>

{

IntPtr clonedConfig;

Interop.WiFi.Config.Clone(configHandle, out clonedConfig);

WiFiConfiguration configItem = new WiFiConfiguration(clonedConfig);

configList.Add(configItem);

return true;

}

return false;

};

int ret = Interop.WiFi.Config.GetForeachConfiguration(GetSafeHandle(), callback, IntPtr.Zero);

CheckReturnValue(ret, "GetForeachConfiguration", PrivilegeNetworkProfile);

return configList;

}

Interop.WiFi.AP.Clone(out IntPtr clonedHandle, apHandle);

return new WiFiAP(clonedHandle);

});

@gemini-code-assist gemini-code-assist Bot Jun 7, 2026

Copy link

Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

 apHandle =>
 {
 int ret = Interop.WiFi.AP.Clone(out IntPtr clonedHandle, apHandle);
 if (ret != (int)WiFiError.None)
 {
 WiFiErrorFactory.ThrowWiFiException(ret, "Failed to clone AP handle");
 }
 return new WiFiAP(clonedHandle);
 });

src/Tizen.Network.WiFi/Tizen.Network.WiFi/WiFiManagerImpl.cs

Comment on lines +240 to +244

configHandle =>

{

Interop.WiFi.Config.Clone(configHandle, out IntPtr clonedConfig);

return new WiFiConfiguration(clonedConfig);

});

@gemini-code-assist gemini-code-assist Bot Jun 7, 2026

Copy link

Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The return value of Interop.WiFi.Config.Clone is ignored. If cloning fails, it may return a non-zero error code and leave clonedConfig as IntPtr.Zero or invalid, which will cause downstream failures when the WiFiConfiguration instance is used. It is safer to check the return value and throw an exception if cloning fails.

 configHandle =>
 {
 int ret = Interop.WiFi.Config.Clone(configHandle, out IntPtr clonedConfig);
 if (ret != (int)WiFiError.None)
 {
 WiFiErrorFactory.ThrowWiFiException(ret, "Failed to clone WiFi configuration");
 }
 return new WiFiConfiguration(clonedConfig);
 });

@JoonghyunCho

JoonghyunCho commented Jun 7, 2026

Copy link

Copy Markdown

Member Author

🤖 [AI Review]

Reviewed — no findings.

Scope checked:

All 4 converted methods (GetFoundAPs, GetFoundSpecificAPs, GetFoundBssids, GetWiFiConfigurations) keep their original CheckReturnValue operation name and privilege — PrivilegeNetworkGet for the 3 AP/Bssid getters, PrivilegeNetworkProfile for configurations.
The two divergent native clone signatures are correctly absorbed by the wrap delegate: AP.Clone(out cloned, original) vs Config.Clone(origin, out cloned) — arg order preserved per call site.
Callback contract is identical: wraps + adds the item and returns true for a non-zero handle, false otherwise.
GetSafeHandle() is still resolved once per call, immediately before the native foreach.
All 4 methods are internal and the generic helper is private → no public API surface, so no XML-doc/since_tizen obligation.

No 🔴 critical issues, no 🟡 suggestions to flag.

Automated review — final merge decision rests with human reviewers.


 Address review feedback

37941b2

Harden EnumerateForeach<T>: catch exceptions raised inside the native
foreach callback so they never unwind across the P/Invoke boundary, and
rethrow after the native call returns. Check the return value of every
AP.Clone / Config.Clone call and throw a WiFiException when cloning fails.
Applied-Human-Comments: 3369222130,3369222132,3369222133,3369222134,3369222136

@JoonghyunCho

JoonghyunCho commented Jun 8, 2026

Copy link

Copy Markdown

Member Author

🤖 [AI Review]
Addressed review feedback in commit 37941b2. Summary: EnumerateForeach<T> now wraps the per-item wrap call in a try/catch, captures any exception, stops the iteration, and rethrows it after the native foreach returns so a managed exception never unwinds across the P/Invoke boundary. In addition, every AP.Clone / Config.Clone call now checks its return value and throws a WiFiException (via WiFiErrorFactory.ThrowWiFiException) when cloning fails.

Labels

ai-task API15

1 participant

@JoonghyunCho

Conversation

@JoonghyunCho JoonghyunCho commented Jun 7, 2026

Summary

Changes

Behavior preservation

Mode

Verification

Uh oh!

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

@gemini-code-assist gemini-code-assist Bot Jun 7, 2026

Choose a reason for hiding this comment

Uh oh!

@gemini-code-assist gemini-code-assist Bot Jun 7, 2026

Choose a reason for hiding this comment

Uh oh!

@gemini-code-assist gemini-code-assist Bot Jun 7, 2026

Choose a reason for hiding this comment

Uh oh!

@gemini-code-assist gemini-code-assist Bot Jun 7, 2026

Choose a reason for hiding this comment

Uh oh!

@gemini-code-assist gemini-code-assist Bot Jun 7, 2026

Choose a reason for hiding this comment

Uh oh!

JoonghyunCho commented Jun 7, 2026

Uh oh!

JoonghyunCho commented Jun 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant