Skip to content

Commit 5956dc6

Browse files
rmarinhoCopilot
andcommitted
Address bot review: exit code check, typed catch, shell doc
- ListAvdNamesAsync: check exit code from emulator -list-avds - TryKillProcess: change bare catch to catch(Exception ex) with logger - TryKillProcess: make instance method to access logger field - RunShellCommandAsync: add XML doc warning about shell interpretation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent a3e6c36 commit 5956dc6

2 files changed

Lines changed: 11 additions & 3 deletions

File tree

src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,11 @@ public async Task StopEmulatorAsync (string serial, CancellationToken cancellati
161161
/// Runs a shell command on a device via 'adb -s &lt;serial&gt; shell &lt;command&gt;'.
162162
/// Returns the full stdout output trimmed, or <c>null</c> on failure.
163163
/// </summary>
164+
/// <remarks>
165+
/// The <paramref name="command"/> is passed as a single argument to <c>adb shell</c>,
166+
/// which means the device's shell interprets it (shell expansion, pipes, semicolons are active).
167+
/// Do not pass untrusted or user-supplied input without proper validation.
168+
/// </remarks>
164169
public virtual async Task<string?> RunShellCommandAsync (string serial, string command, CancellationToken cancellationToken = default)
165170
{
166171
using var stdout = new StringWriter ();

src/Xamarin.Android.Tools.AndroidSdk/Runners/EmulatorRunner.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,12 @@ public Process LaunchAvd (string avdName, bool coldBoot = false, IEnumerable<str
9797
public async Task<IReadOnlyList<string>> ListAvdNamesAsync (CancellationToken cancellationToken = default)
9898
{
9999
using var stdout = new StringWriter ();
100+
using var stderr = new StringWriter ();
100101
var psi = ProcessUtils.CreateProcessStartInfo (emulatorPath, "-list-avds");
101102

102103
logger?.Invoke (TraceLevel.Verbose, "Running: emulator -list-avds");
103-
await ProcessUtils.StartProcess (psi, stdout, null, cancellationToken, environmentVariables).ConfigureAwait (false);
104+
var exitCode = await ProcessUtils.StartProcess (psi, stdout, stderr, cancellationToken, environmentVariables).ConfigureAwait (false);
105+
ProcessUtils.ThrowIfFailed (exitCode, "emulator -list-avds", stderr);
104106

105107
return ParseListAvdsOutput (stdout.ToString ());
106108
}
@@ -243,16 +245,17 @@ public async Task<EmulatorBootResult> BootAvdAsync (
243245
return null;
244246
}
245247

246-
static void TryKillProcess (Process process)
248+
void TryKillProcess (Process process)
247249
{
248250
try {
249251
#if NET5_0_OR_GREATER
250252
process.Kill (entireProcessTree: true);
251253
#else
252254
process.Kill ();
253255
#endif
254-
} catch {
256+
} catch (Exception ex) {
255257
// Best-effort: process may have already exited
258+
logger?.Invoke (TraceLevel.Verbose, $"Failed to stop emulator process: {ex.Message}");
256259
} finally {
257260
process.Dispose ();
258261
}

0 commit comments

Comments
 (0)