Team2890/allwpilib
4
0
Fork 0
mirror of https://github.com/wpilibsuite/allwpilib synced 2026-06-23 01:21:42 +00:00
Code Issues Packages Projects Releases Wiki Activity

Remove memory leak in ConditionalCommand (#537)

Browse Source 
This also properly handles nullptrs passed into ConditionalCommand
instead of having Undefined Behavior or NullPointerExceptions.
This commit is contained in:
Chris Gregory
2017-06-30 22:01:21 -07:00
committed by Peter Johnson
parent 4fd4a50d41
commit d2de94778e
3 changed files with 49 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
wpilibc/shared/include/Commands/ConditionalCommand.h
View File

@@ -40,10 +40,9 @@ namespace frc {
*/
class ConditionalCommand : public Command {
public:
explicit ConditionalCommand(Command* onTrue,
Command* onFalse = new InstantCommand());
explicit ConditionalCommand(Command* onTrue, Command* onFalse = nullptr);
ConditionalCommand(const std::string& name, Command* onTrue,
Command* onFalse = new InstantCommand());
Command* onFalse = nullptr);
virtual ~ConditionalCommand() = default;
protected:

31
wpilibc/shared/src/Commands/ConditionalCommand.cpp
View File

@@ -11,6 +11,17 @@
using namespace frc;
static void RequireAll(Command& command, Command* onTrue, Command* onFalse) {
if (onTrue != nullptr) {
for (auto requirement : onTrue->GetRequirements())
command.Requires(requirement);
}
if (onFalse != nullptr) {
for (auto requirement : onFalse->GetRequirements())
command.Requires(requirement);
}
}
/**
* Creates a new ConditionalCommand with given onTrue and onFalse Commands.
*
@@ -23,8 +34,7 @@ ConditionalCommand::ConditionalCommand(Command* onTrue, Command* onFalse) {
m_onTrue = onTrue;
m_onFalse = onFalse;
for (auto requirement : m_onTrue->GetRequirements()) Requires(requirement);
for (auto requirement : m_onFalse->GetRequirements()) Requires(requirement);
RequireAll(*this, onTrue, onFalse);
}
/**
@@ -42,8 +52,7 @@ ConditionalCommand::ConditionalCommand(const std::string& name, Command* onTrue,
m_onTrue = onTrue;
m_onFalse = onFalse;
for (auto requirement : m_onTrue->GetRequirements()) Requires(requirement);
for (auto requirement : m_onFalse->GetRequirements()) Requires(requirement);
RequireAll(*this, onTrue, onFalse);
}
void ConditionalCommand::_Initialize() {
@@ -53,13 +62,15 @@ void ConditionalCommand::_Initialize() {
m_chosenCommand = m_onFalse;
}
/*
* This is a hack to make cancelling the chosen command inside a CommandGroup
* work properly
*/
m_chosenCommand->ClearRequirements();
if (m_chosenCommand != nullptr) {
/*
* This is a hack to make cancelling the chosen command inside a
* CommandGroup work properly
*/
m_chosenCommand->ClearRequirements();
m_chosenCommand->Start();
m_chosenCommand->Start();
}
}
void ConditionalCommand::_Cancel() {

45
wpilibj/src/shared/java/edu/wpi/first/wpilibj/command/ConditionalCommand.java
View File

@@ -47,6 +47,20 @@ public abstract class ConditionalCommand extends Command {
*/
private Command m_chosenCommand = null;
private void requireAll() {
if (m_onTrue != null) {
for (Enumeration e = m_onTrue.getRequirements(); e.hasMoreElements(); ) {
requires((Subsystem) e.nextElement());
}
}
if (m_onFalse != null) {
for (Enumeration e = m_onFalse.getRequirements(); e.hasMoreElements(); ) {
requires((Subsystem) e.nextElement());
}
}
}
/**
* Creates a new ConditionalCommand with given onTrue and onFalse Commands.
*
@@ -55,7 +69,7 @@ public abstract class ConditionalCommand extends Command {
* @param onTrue The Command to execute if {@link ConditionalCommand#condition()} returns true
*/
public ConditionalCommand(Command onTrue) {
this(onTrue, new InstantCommand());
this(onTrue, null);
}
/**
@@ -70,13 +84,7 @@ public abstract class ConditionalCommand extends Command {
m_onTrue = onTrue;
m_onFalse = onFalse;
for (Enumeration e = m_onTrue.getRequirements(); e.hasMoreElements(); ) {
requires((Subsystem) e.nextElement());
}
for (Enumeration e = m_onFalse.getRequirements(); e.hasMoreElements(); ) {
requires((Subsystem) e.nextElement());
}
requireAll();
}
/**
@@ -88,7 +96,7 @@ public abstract class ConditionalCommand extends Command {
* @param onTrue The Command to execute if {@link ConditionalCommand#condition()} returns true
*/
public ConditionalCommand(String name, Command onTrue) {
this(name, onTrue, new InstantCommand());
this(name, onTrue, null);
}
/**
@@ -105,13 +113,7 @@ public abstract class ConditionalCommand extends Command {
m_onTrue = onTrue;
m_onFalse = onFalse;
for (Enumeration e = m_onTrue.getRequirements(); e.hasMoreElements(); ) {
requires((Subsystem) e.nextElement());
}
for (Enumeration e = m_onFalse.getRequirements(); e.hasMoreElements(); ) {
requires((Subsystem) e.nextElement());
}
requireAll();
}
/**
@@ -132,10 +134,15 @@ public abstract class ConditionalCommand extends Command {
m_chosenCommand = m_onFalse;
}
// This is a hack to make cancelling the chosen command inside a CommandGroup work properly
m_chosenCommand.clearRequirements();
if (m_chosenCommand != null) {
/*
* This is a hack to make cancelling the chosen command inside a
* CommandGroup work properly
*/
m_chosenCommand.clearRequirements();
m_chosenCommand.start();
m_chosenCommand.start();
}
}
@Override