+semver:minor Explicitly set UseShellExecute, .net core default changed#1457
+semver:minor Explicitly set UseShellExecute, .net core default changed#1457andrew-polk merged 1 commit intomasterfrom
Conversation
|
I don't expect this to break any of my software, but I'm curious: it looks like every single change notes that we didn't investigate whether the change was needed in that place. I would have expected there to be at least one pace where the change was being made because of a known need. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // UseShellExecute defaults to true in .net framework (including .net 4) and to false in .net core (including .net 8) | ||
| // We are explicitly setting it to true for consistency with the old behavior | ||
| // but have not checked if it is necessary here. | ||
| var prs = new Process { StartInfo = { FileName = exePath, Arguments = args, UseShellExecute = true } }; |
Check warning
Code scanning / CodeQL
Missing Dispose call on local IDisposable Warning
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the missing Dispose call for the local Process object, ensure that the object is disposed of as soon as it is no longer needed. The best practice is to wrap its lifetime in a using statement, which guarantees disposal even in case of exceptions. If LaunchArchivingProgram(prs) returns only after the process object is no longer needed (i.e., does not retain a reference), wrap the construction and usage in a using block here. Otherwise, ensure disposal happens in LaunchArchivingProgram. Since we only control the lines shown here and CodeQL flags this site, it is best to use a local using statement:
Change:
176: var prs = new Process { StartInfo = { FileName = exePath, Arguments = args, UseShellExecute = true } };
177:
178: LaunchArchivingProgram(prs);to:
176: using (var prs = new Process { StartInfo = { FileName = exePath, Arguments = args, UseShellExecute = true } })
177: {
178: LaunchArchivingProgram(prs);
179: }This ensures prs is disposed after LaunchArchivingProgram returns.
No extra imports or method definitions are necessary, just the use of a using block.
| @@ -173,9 +173,10 @@ | ||
| // UseShellExecute defaults to true in .net framework (including .net 4) and to false in .net core (including .net 8) | ||
| // We are explicitly setting it to true for consistency with the old behavior | ||
| // but have not checked if it is necessary here. | ||
| var prs = new Process { StartInfo = { FileName = exePath, Arguments = args, UseShellExecute = true } }; | ||
|
|
||
| LaunchArchivingProgram(prs); | ||
| using (var prs = new Process { StartInfo = { FileName = exePath, Arguments = args, UseShellExecute = true } }) | ||
| { | ||
| LaunchArchivingProgram(prs); | ||
| } | ||
| } | ||
|
|
||
| #region Create IMDI package asynchronously |
nabalone
force-pushed
the
netCore_useShellExecute
branch
from
0e991e9 to
b1f4e2a
Compare
nabalone
left a comment
nabalone
left a comment
There was a problem hiding this comment.
My mistake. I have corrected the comments
Reviewable status: 0 of 16 files reviewed, 1 unresolved discussion
tombogle
left a comment
tombogle
left a comment
There was a problem hiding this comment.
@tombogle reviewed 15 of 16 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @nabalone)
nabalone
force-pushed
the
netCore_useShellExecute
branch
from
b1f4e2a to
36ff067
Compare
3 participants
UseShellExecute defaults to true in .net framework (including .net 4) and to false in .net core (including .net 8). Set it explicitly for consistency
This change is