Skip to content

Commit

Permalink
Bug 1733821 - [9/9] Drive-by cleanup: add nullptr_t overload for Chec…
Browse files Browse the repository at this point in the history
…kArg r=nika

Since `CheckArg`'s `aParam` is declared as `const CharT **`, it's
treated as a deduction parameter. Unfortunately, `nullptr` is of type
`nullptr_t`, and it doesn't get coerced before template argument
deduction takes place. To allow passing `nullptr`, people have been
using unwieldy constructs like `static_cast<const wchar_t**>(nullptr)`
when `aParam` isn't needed.

Centralize this by adding an overload of `CheckArg` that explicitly
takes `nullptr_t` and forwards it on to the primary implementation.
Strip out all the now-unnecessary `static_cast`s everywhere else.

No functional changes.

Differential Revision: https://phabricator.services.mozilla.com/D152327
  • Loading branch information
selenography committed Aug 2, 2022
1 parent 8ae15d7 commit bc46f84
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 26 deletions.
4 changes: 1 addition & 3 deletions browser/app/winlauncher/LaunchUnelevated.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,7 @@ LauncherVoidResult LaunchUnelevated(int aArgc, wchar_t* aArgv[]) {
// This should have already been checked, but just in case...
EnsureBrowserCommandlineSafe(aArgc, aArgv);

if (mozilla::CheckArg(aArgc, aArgv, "osint",
static_cast<const wchar_t**>(nullptr),
CheckArgFlag::None)) {
if (mozilla::CheckArg(aArgc, aArgv, "osint", nullptr, CheckArgFlag::None)) {
// If the command line contains -osint, we have to arrange things in a
// particular order.
//
Expand Down
30 changes: 11 additions & 19 deletions browser/app/winlauncher/LauncherProcessWin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,21 +146,16 @@ static void SetMitigationPolicies(mozilla::ProcThreadAttributes& aAttrs,
static mozilla::LauncherFlags ProcessCmdLine(int& aArgc, wchar_t* aArgv[]) {
mozilla::LauncherFlags result = mozilla::LauncherFlags::eNone;

if (mozilla::CheckArg(aArgc, aArgv, "wait-for-browser",
static_cast<const wchar_t**>(nullptr),
if (mozilla::CheckArg(aArgc, aArgv, "wait-for-browser", nullptr,
mozilla::CheckArgFlag::RemoveArg) ==
mozilla::ARG_FOUND ||
mozilla::CheckArg(aArgc, aArgv, "marionette",
static_cast<const wchar_t**>(nullptr),
mozilla::CheckArg(aArgc, aArgv, "marionette", nullptr,
mozilla::CheckArgFlag::None) == mozilla::ARG_FOUND ||
mozilla::CheckArg(aArgc, aArgv, "backgroundtask",
static_cast<const wchar_t**>(nullptr),
mozilla::CheckArg(aArgc, aArgv, "backgroundtask", nullptr,
mozilla::CheckArgFlag::None) == mozilla::ARG_FOUND ||
mozilla::CheckArg(aArgc, aArgv, "headless",
static_cast<const wchar_t**>(nullptr),
mozilla::CheckArg(aArgc, aArgv, "headless", nullptr,
mozilla::CheckArgFlag::None) == mozilla::ARG_FOUND ||
mozilla::CheckArg(aArgc, aArgv, "remote-debugging-port",
static_cast<const wchar_t**>(nullptr),
mozilla::CheckArg(aArgc, aArgv, "remote-debugging-port", nullptr,
mozilla::CheckArgFlag::None) == mozilla::ARG_FOUND ||
mozilla::EnvHasValue("MOZ_AUTOMATION") ||
mozilla::EnvHasValue("MOZ_HEADLESS")) {
Expand Down Expand Up @@ -220,9 +215,9 @@ static bool DoLauncherProcessChecks(int& argc, wchar_t** argv) {
result = true;
}

result |= mozilla::CheckArg(
argc, argv, "launcher", static_cast<const wchar_t**>(nullptr),
mozilla::CheckArgFlag::RemoveArg) == mozilla::ARG_FOUND;
result |=
mozilla::CheckArg(argc, argv, "launcher", nullptr,
mozilla::CheckArgFlag::RemoveArg) == mozilla::ARG_FOUND;

return result;
}
Expand All @@ -238,8 +233,7 @@ static mozilla::Maybe<bool> RunAsLauncherProcess(int& argc, wchar_t** argv) {
#if defined(MOZ_LAUNCHER_PROCESS)
bool forceLauncher =
runAsLauncher &&
mozilla::CheckArg(argc, argv, "force-launcher",
static_cast<const wchar_t**>(nullptr),
mozilla::CheckArg(argc, argv, "force-launcher", nullptr,
mozilla::CheckArgFlag::RemoveArg) == mozilla::ARG_FOUND;

mozilla::LauncherRegistryInfo::ProcessType desiredType =
Expand Down Expand Up @@ -279,17 +273,15 @@ Maybe<int> LauncherMain(int& argc, wchar_t* argv[],

SetLauncherErrorAppData(aAppData);

if (CheckArg(argc, argv, "log-launcher-error",
static_cast<const wchar_t**>(nullptr),
if (CheckArg(argc, argv, "log-launcher-error", nullptr,
mozilla::CheckArgFlag::RemoveArg) == ARG_FOUND) {
SetLauncherErrorForceEventLog();
}

// return fast when we're a child process.
// (The remainder of this function has some side effects that are
// undesirable for content processes)
if (mozilla::CheckArg(argc, argv, "contentproc",
static_cast<const wchar_t**>(nullptr),
if (mozilla::CheckArg(argc, argv, "contentproc", nullptr,
mozilla::CheckArgFlag::None) == mozilla::ARG_FOUND) {
// A child process should not instantiate LauncherRegistryInfo.
return Nothing();
Expand Down
12 changes: 10 additions & 2 deletions toolkit/xre/CmdLineAndEnvUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,14 @@ inline ArgResult CheckArg(int& aArgc, CharT** aArgv, const char* aArg,
return ar;
}

template <typename CharT>
inline ArgResult CheckArg(int& aArgc, CharT** aArgv, const char* aArg,
std::nullptr_t,
CheckArgFlag aFlags = CheckArgFlag::RemoveArg) {
return CheckArg<CharT>(aArgc, aArgv, aArg,
static_cast<const CharT**>(nullptr), aFlags);
}

namespace internal {
// template <typename T>
// constexpr bool IsStringRange =
Expand Down Expand Up @@ -248,8 +256,8 @@ inline bool EnsureCommandlineSafeImpl(int aArgc, CharT** aArgv,

// If "-osint" (or the equivalent) is not present, then this is trivially
// satisfied.
if (CheckArg(aArgc, aArgv, osintLit, static_cast<const CharT**>(nullptr),
CheckArgFlag::None) != ARG_FOUND) {
if (CheckArg(aArgc, aArgv, osintLit, nullptr, CheckArgFlag::None) !=
ARG_FOUND) {
return true;
}

Expand Down
3 changes: 1 addition & 2 deletions toolkit/xre/SafeMode.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ inline Maybe<bool> IsSafeModeRequested(
checkArgFlags |= CheckArgFlag::RemoveArg;
}

ArgResult ar = CheckArg(aArgc, aArgv, "safe-mode",
static_cast<const CharT**>(nullptr), checkArgFlags);
ArgResult ar = CheckArg(aArgc, aArgv, "safe-mode", nullptr, checkArgFlags);
if (ar == ARG_BAD) {
return Nothing();
}
Expand Down

0 comments on commit bc46f84

Please sign in to comment.