fix std:clamp usage

We appear to consistently assume that clamp can be called with 0, size as
interval, but that actually results in a possible out of bounds access at the
upper end.

Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This commit is contained in:
Dirk Hohndel 2025-01-10 15:44:49 -08:00 committed by Michael Keller
parent bbff369bf2
commit 5671a634a8
4 changed files with 10 additions and 10 deletions

View file

@ -158,7 +158,7 @@ stats_summary calculate_stats_summary(bool selected_only)
process_dive(*dp, out.stats_by_depth[0]); process_dive(*dp, out.stats_by_depth[0]);
int d_idx = dp->maxdepth.mm / (STATS_DEPTH_BUCKET * 1000); int d_idx = dp->maxdepth.mm / (STATS_DEPTH_BUCKET * 1000);
d_idx = std::clamp(d_idx, 0, STATS_MAX_DEPTH / STATS_DEPTH_BUCKET); d_idx = std::clamp(d_idx, 0, STATS_MAX_DEPTH / STATS_DEPTH_BUCKET - 1);
process_dive(*dp, out.stats_by_depth[d_idx + 1]); process_dive(*dp, out.stats_by_depth[d_idx + 1]);
out.stats_by_depth[d_idx + 1].selection_size++; out.stats_by_depth[d_idx + 1].selection_size++;
@ -167,7 +167,7 @@ stats_summary calculate_stats_summary(bool selected_only)
process_dive(*dp, out.stats_by_temp[0]); process_dive(*dp, out.stats_by_temp[0]);
int t_idx = ((int)mkelvin_to_C(dp->mintemp.mkelvin)) / STATS_TEMP_BUCKET; int t_idx = ((int)mkelvin_to_C(dp->mintemp.mkelvin)) / STATS_TEMP_BUCKET;
t_idx = std::clamp(t_idx, 0, STATS_MAX_TEMP / STATS_TEMP_BUCKET); t_idx = std::clamp(t_idx, 0, STATS_MAX_TEMP / STATS_TEMP_BUCKET - 1);
process_dive(*dp, out.stats_by_temp[t_idx + 1]); process_dive(*dp, out.stats_by_temp[t_idx + 1]);
out.stats_by_temp[t_idx + 1].selection_size++; out.stats_by_temp[t_idx + 1].selection_size++;

View file

@ -81,7 +81,7 @@ void StatsManager::var1Changed(int idx)
{ {
if (uiState.var1.variables.empty()) if (uiState.var1.variables.empty())
return; return;
idx = std::clamp(idx, 0, (int)uiState.var1.variables.size()); idx = std::clamp(idx, 0, (int)uiState.var1.variables.size() - 1);
state.var1Changed(uiState.var1.variables[idx].id); state.var1Changed(uiState.var1.variables[idx].id);
updateUi(); updateUi();
} }
@ -96,7 +96,7 @@ void StatsManager::var2Changed(int idx)
{ {
if (uiState.var2.variables.empty()) if (uiState.var2.variables.empty())
return; return;
idx = std::clamp(idx, 0, (int)uiState.var2.variables.size()); idx = std::clamp(idx, 0, (int)uiState.var2.variables.size() - 1);
state.var2Changed(uiState.var2.variables[idx].id); state.var2Changed(uiState.var2.variables[idx].id);
updateUi(); updateUi();
} }
@ -111,7 +111,7 @@ void StatsManager::var2OperationChanged(int idx)
{ {
if (uiState.operations2.variables.empty()) if (uiState.operations2.variables.empty())
return; return;
idx = std::clamp(idx, 0, (int)uiState.operations2.variables.size()); idx = std::clamp(idx, 0, (int)uiState.operations2.variables.size() - 1);
state.var2OperationChanged(uiState.operations2.variables[idx].id); state.var2OperationChanged(uiState.operations2.variables[idx].id);
updateUi(); updateUi();
} }
@ -120,7 +120,7 @@ void StatsManager::sortMode1Changed(int idx)
{ {
if (uiState.sortMode1.variables.empty()) if (uiState.sortMode1.variables.empty())
return; return;
idx = std::clamp(idx, 0, (int)uiState.sortMode1.variables.size()); idx = std::clamp(idx, 0, (int)uiState.sortMode1.variables.size() - 1);
state.sortMode1Changed(uiState.sortMode1.variables[idx].id); state.sortMode1Changed(uiState.sortMode1.variables[idx].id);
updateUi(); updateUi();
} }

View file

@ -41,7 +41,7 @@ void ChartListModel::initIcon(ChartSubType type, const char *name, int iconSize)
const QPixmap &ChartListModel::getIcon(ChartSubType type, bool warning) const const QPixmap &ChartListModel::getIcon(ChartSubType type, bool warning) const
{ {
int idx = std::clamp((int)type, 0, (int)ChartSubType::Count); int idx = std::clamp((int)type, 0, (int)ChartSubType::Count - 1);
return warning ? subTypeIcons[idx].warning : subTypeIcons[idx].normal; return warning ? subTypeIcons[idx].warning : subTypeIcons[idx].normal;
} }

View file

@ -256,7 +256,7 @@ static ChartValidity chartValidity(const ChartTypeDesc &desc,
if ((operation != StatsOperation::Invalid) != desc.var2HasOperations) if ((operation != StatsOperation::Invalid) != desc.var2HasOperations)
return ChartValidity::Invalid; return ChartValidity::Invalid;
return valid1 == ChartValidity::Undesired || valid2 == ChartValidity::Undesired ? return valid1 == ChartValidity::Undesired || valid2 == ChartValidity::Undesired ?
ChartValidity::Undesired : ChartValidity::Good; ChartValidity::Undesired : ChartValidity::Good;
} }
@ -454,7 +454,7 @@ static const StatsBinner *idxToBinner(const StatsVariable *v, int idx, bool seco
void StatsState::var1Changed(int id) void StatsState::var1Changed(int id)
{ {
var1 = stats_variables[std::clamp(id, 0, (int)stats_variables.size())]; var1 = stats_variables[std::clamp(id, 0, (int)stats_variables.size() - 1)];
validate(true); validate(true);
} }
@ -478,7 +478,7 @@ void StatsState::var2Changed(int id)
{ {
// The "none" variable is represented by a nullptr // The "none" variable is represented by a nullptr
var2 = id == none_idx ? nullptr var2 = id == none_idx ? nullptr
: stats_variables[std::clamp(id, 0, (int)stats_variables.size())]; : stats_variables[std::clamp(id, 0, (int)stats_variables.size() - 1)];
validate(true); validate(true);
} }