Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[chip-tool] Returns an error if an unknown argument is passed to the … #24875

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions examples/chip-tool/commands/common/Commands.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,21 +41,27 @@ constexpr const char * kJsonCommandKey = "command";
constexpr const char * kJsonCommandSpecifierKey = "command_specifier";
constexpr const char * kJsonArgumentsKey = "arguments";

std::vector<std::string> GetArgumentsFromJson(Command * command, Json::Value & value, bool optional)
bool GetArgumentsFromJson(Command * command, Json::Value & value, bool optional, std::vector<std::string> & outArgs)
{
auto memberNames = value.getMemberNames();

std::vector<std::string> args;
for (size_t i = 0; i < command->GetArgumentsCount(); i++)
{
auto argName = command->GetArgumentName(i);
for (auto const & memberName : value.getMemberNames())
auto argName = command->GetArgumentName(i);
auto memberNamesIterator = memberNames.begin();
while (memberNamesIterator != memberNames.end())
{
auto memberName = *memberNamesIterator;
if (strcasecmp(argName, memberName.c_str()) != 0)
{
memberNamesIterator++;
continue;
}

if (command->GetArgumentIsOptional(i) != optional)
{
memberNamesIterator = memberNames.erase(memberNamesIterator);
continue;
}

Expand All @@ -66,10 +72,20 @@ std::vector<std::string> GetArgumentsFromJson(Command * command, Json::Value & v

auto argValue = value[memberName].asString();
args.push_back(std::move(argValue));
memberNamesIterator = memberNames.erase(memberNamesIterator);
break;
}
}
return args;

if (memberNames.size())
{
auto memberName = memberNames.front();
ChipLogError(chipTool, "The argument \"\%s\" is not supported.", memberName.c_str());
return false;
}

outArgs = args;
return true;
};

// Check for arguments with a starting '"' but no ending '"': those
Expand Down Expand Up @@ -542,8 +558,10 @@ bool Commands::DecodeArgumentsFromBase64EncodedJson(const char * json, std::vect
VerifyOrReturnValue(parsedArguments, false, ChipLogError(chipTool, "Error while parsing json."));
VerifyOrReturnValue(jsonArguments.isObject(), false, ChipLogError(chipTool, "Unexpected json type, expects and object."));

auto mandatoryArguments = GetArgumentsFromJson(command, jsonArguments, false /* addOptional */);
auto optionalArguments = GetArgumentsFromJson(command, jsonArguments, true /* addOptional */);
std::vector<std::string> mandatoryArguments;
std::vector<std::string> optionalArguments;
VerifyOrReturnValue(GetArgumentsFromJson(command, jsonArguments, false /* addOptional */, mandatoryArguments), false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not for this PR, but eventually we may want to consider that boolean is often not ideal in mixed arguments ... since you have to specify addOptional anyway for readability, we could enum this to OptionalArguments::kInclude / OptionalArguments::kSkip or similar.

VerifyOrReturnValue(GetArgumentsFromJson(command, jsonArguments, true /* addOptional */, optionalArguments), false);

args.push_back(std::move(clusterName));
args.push_back(std::move(commandName));
Expand Down