Skip to content

Commit 4cf2f8b

Browse files
rouaultrouault
committed
GDALInConstructionAlgorithmArg::SetDefault(): stricter type checking
Fixes Coverity 1592298 "uncaught exception"
1 parent 19671ee commit 4cf2f8b

File tree

2 files changed

+260
-67
lines changed

2 files changed

+260
-67
lines changed

‎autotest/cpp/test_gdal_algorithm.cpp

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,134 @@ TEST_F(test_gdal_algorithm, GDALAlgorithmArgDecl_SetMaxCount)
101101
2);
102102
}
103103

104+
TEST_F(test_gdal_algorithm, GDALAlgorithmArgDecl_SetDefault)
105+
{
106+
{
107+
auto decl = GDALAlgorithmArgDecl("", 0, "", GAAT_BOOLEAN);
108+
decl.SetDefault(true);
109+
EXPECT_TRUE(decl.GetDefault<bool>());
110+
111+
CPLErrorStateBackuper oBackuper(CPLQuietErrorHandler);
112+
CPLErrorReset();
113+
decl.SetDefault("invalid");
114+
EXPECT_EQ(CPLGetLastErrorType(), CE_Failure);
115+
}
116+
117+
{
118+
auto decl = GDALAlgorithmArgDecl("", 0, "", GAAT_INTEGER);
119+
decl.SetDefault(5);
120+
EXPECT_EQ(decl.GetDefault<int>(), 5);
121+
122+
CPLErrorStateBackuper oBackuper(CPLQuietErrorHandler);
123+
CPLErrorReset();
124+
decl.SetDefault("invalid");
125+
EXPECT_EQ(CPLGetLastErrorType(), CE_Failure);
126+
}
127+
128+
{
129+
auto decl = GDALAlgorithmArgDecl("", 0, "", GAAT_REAL);
130+
decl.SetDefault(4.5);
131+
EXPECT_EQ(decl.GetDefault<double>(), 4.5);
132+
133+
decl.SetDefault(5);
134+
EXPECT_EQ(decl.GetDefault<double>(), 5);
135+
136+
decl.SetDefault(2.5f);
137+
EXPECT_EQ(decl.GetDefault<double>(), 2.5);
138+
139+
CPLErrorStateBackuper oBackuper(CPLQuietErrorHandler);
140+
CPLErrorReset();
141+
decl.SetDefault("invalid");
142+
EXPECT_EQ(CPLGetLastErrorType(), CE_Failure);
143+
}
144+
145+
{
146+
auto decl = GDALAlgorithmArgDecl("", 0, "", GAAT_STRING);
147+
148+
decl.SetDefault("ab");
149+
EXPECT_STREQ(decl.GetDefault<std::string>().c_str(), "ab");
150+
151+
decl.SetDefault(std::string("cd"));
152+
EXPECT_STREQ(decl.GetDefault<std::string>().c_str(), "cd");
153+
154+
CPLErrorStateBackuper oBackuper(CPLQuietErrorHandler);
155+
CPLErrorReset();
156+
decl.SetDefault(0);
157+
EXPECT_EQ(CPLGetLastErrorType(), CE_Failure);
158+
}
159+
160+
{
161+
auto decl = GDALAlgorithmArgDecl("", 0, "", GAAT_INTEGER_LIST);
162+
decl.SetDefault(5);
163+
std::vector<int> expected{5};
164+
EXPECT_EQ(decl.GetDefault<std::vector<int>>(), expected);
165+
166+
CPLErrorStateBackuper oBackuper(CPLQuietErrorHandler);
167+
CPLErrorReset();
168+
decl.SetDefault("invalid");
169+
EXPECT_EQ(CPLGetLastErrorType(), CE_Failure);
170+
}
171+
172+
{
173+
auto decl = GDALAlgorithmArgDecl("", 0, "", GAAT_REAL_LIST);
174+
decl.SetDefault(4.5);
175+
{
176+
std::vector<double> expected{4.5};
177+
EXPECT_EQ(decl.GetDefault<std::vector<double>>(), expected);
178+
}
179+
180+
decl.SetDefault(5);
181+
{
182+
std::vector<double> expected{5};
183+
EXPECT_EQ(decl.GetDefault<std::vector<double>>(), expected);
184+
}
185+
186+
decl.SetDefault(2.5f);
187+
{
188+
std::vector<double> expected{2.5};
189+
EXPECT_EQ(decl.GetDefault<std::vector<double>>(), expected);
190+
}
191+
192+
CPLErrorStateBackuper oBackuper(CPLQuietErrorHandler);
193+
CPLErrorReset();
194+
decl.SetDefault("invalid");
195+
EXPECT_EQ(CPLGetLastErrorType(), CE_Failure);
196+
}
197+
198+
{
199+
auto decl = GDALAlgorithmArgDecl("", 0, "", GAAT_STRING_LIST);
200+
201+
decl.SetDefault("ab");
202+
{
203+
std::vector<std::string> expected{"ab"};
204+
EXPECT_EQ(decl.GetDefault<std::vector<std::string>>(), expected);
205+
}
206+
207+
CPLErrorStateBackuper oBackuper(CPLQuietErrorHandler);
208+
CPLErrorReset();
209+
decl.SetDefault(0);
210+
EXPECT_EQ(CPLGetLastErrorType(), CE_Failure);
211+
}
212+
213+
{
214+
auto decl = GDALAlgorithmArgDecl("", 0, "", GAAT_DATASET);
215+
216+
CPLErrorStateBackuper oBackuper(CPLQuietErrorHandler);
217+
CPLErrorReset();
218+
decl.SetDefault(0);
219+
EXPECT_EQ(CPLGetLastErrorType(), CE_Failure);
220+
}
221+
222+
{
223+
auto decl = GDALAlgorithmArgDecl("", 0, "", GAAT_DATASET_LIST);
224+
225+
CPLErrorStateBackuper oBackuper(CPLQuietErrorHandler);
226+
CPLErrorReset();
227+
decl.SetDefault(0);
228+
EXPECT_EQ(CPLGetLastErrorType(), CE_Failure);
229+
}
230+
}
231+
104232
TEST_F(test_gdal_algorithm, GDALAlgorithmArg_Set)
105233
{
106234
{

‎gcore/gdalalgorithm.h

Lines changed: 132 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -618,54 +618,108 @@ class CPL_DLL GDALAlgorithmArgDecl final
618618
template <class T> GDALAlgorithmArgDecl &SetDefault(const T &value)
619619
{
620620
m_hasDefaultValue = true;
621-
if constexpr (std::is_same_v<T, std::string>)
622-
{
623-
if (m_type == GAAT_STRING_LIST)
624-
{
625-
m_defaultValue = std::vector<std::string>{value};
626-
return *this;
627-
}
628-
}
629-
else if constexpr (std::is_same_v<T, int>)
630-
{
631-
if (m_type == GAAT_REAL)
632-
{
633-
m_defaultValue = static_cast<double>(value);
634-
return *this;
635-
}
636-
else if (m_type == GAAT_INTEGER_LIST)
637-
{
638-
m_defaultValue = std::vector<int>{value};
639-
return *this;
640-
}
641-
else if (m_type == GAAT_REAL_LIST)
642-
{
643-
m_defaultValue =
644-
std::vector<double>{static_cast<double>(value)};
645-
return *this;
646-
}
647-
}
648-
else if constexpr (std::is_same_v<T, double>)
621+
try
649622
{
650-
if (m_type == GAAT_REAL_LIST)
623+
switch (m_type)
651624
{
652-
m_defaultValue = std::vector<double>{value};
653-
return *this;
625+
case GAAT_BOOLEAN:
626+
{
627+
if constexpr (std::is_same_v<T, bool>)
628+
{
629+
m_defaultValue = value;
630+
return *this;
631+
}
632+
break;
633+
}
634+
635+
case GAAT_STRING:
636+
{
637+
if constexpr (std::is_same_v<T, std::string>)
638+
{
639+
m_defaultValue = value;
640+
return *this;
641+
}
642+
break;
643+
}
644+
645+
case GAAT_INTEGER:
646+
{
647+
if constexpr (std::is_same_v<T, int>)
648+
{
649+
m_defaultValue = value;
650+
return *this;
651+
}
652+
break;
653+
}
654+
655+
case GAAT_REAL:
656+
{
657+
if constexpr (std::is_same_v<T, double> ||
658+
std::is_same_v<T, float> ||
659+
std::is_same_v<T, int>)
660+
{
661+
m_defaultValue = static_cast<double>(value);
662+
return *this;
663+
}
664+
break;
665+
}
666+
667+
case GAAT_STRING_LIST:
668+
{
669+
if constexpr (std::is_same_v<T, std::string>)
670+
{
671+
m_defaultValue = std::vector<std::string>{value};
672+
return *this;
673+
}
674+
break;
675+
}
676+
677+
case GAAT_INTEGER_LIST:
678+
{
679+
if constexpr (std::is_same_v<T, int>)
680+
{
681+
m_defaultValue = std::vector<int>{value};
682+
return *this;
683+
}
684+
break;
685+
}
686+
687+
case GAAT_REAL_LIST:
688+
{
689+
if constexpr (std::is_same_v<T, double> ||
690+
std::is_same_v<T, float> ||
691+
std::is_same_v<T, int>)
692+
{
693+
m_defaultValue =
694+
std::vector<double>{static_cast<double>(value)};
695+
return *this;
696+
}
697+
break;
698+
}
699+
700+
case GAAT_DATASET:
701+
case GAAT_DATASET_LIST:
702+
break;
654703
}
655704
}
656-
try
657-
{
658-
m_defaultValue = value;
659-
}
660705
catch (const std::bad_variant_access &)
661706
{
662-
CPLError(CE_Failure, CPLE_AppDefined,
663-
"Argument %s: SetDefault(): unexpected type for value",
664-
GetName().c_str());
707+
// should not happen
708+
// fallthrough
665709
}
710+
CPLError(CE_Failure, CPLE_AppDefined,
711+
"Argument %s: SetDefault(): unexpected type for value",
712+
GetName().c_str());
666713
return *this;
667714
}
668715

716+
/** Declare a default value for the argument.
717+
*/
718+
GDALAlgorithmArgDecl &SetDefault(const char *value)
719+
{
720+
return SetDefault(std::string(value));
721+
}
722+
669723
/** Declare the minimum number of values for the argument. Defaults to 0.
670724
* Only applies to list type of arguments.
671725
* Setting it to non-zero does *not* make the argument required. It just
@@ -1835,36 +1889,47 @@ class CPL_DLL GDALInConstructionAlgorithmArg final : public GDALAlgorithmArg
18351889
if constexpr (!std::is_same_v<T, GDALArgDatasetValue> &&
18361890
!std::is_same_v<T, std::vector<GDALArgDatasetValue>>)
18371891
{
1838-
switch (m_decl.GetType())
1892+
try
18391893
{
1840-
case GAAT_BOOLEAN:
1841-
*std::get<bool *>(m_value) = m_decl.GetDefault<bool>();
1842-
break;
1843-
case GAAT_STRING:
1844-
*std::get<std::string *>(m_value) =
1845-
m_decl.GetDefault<std::string>();
1846-
break;
1847-
case GAAT_INTEGER:
1848-
*std::get<int *>(m_value) = m_decl.GetDefault<int>();
1849-
break;
1850-
case GAAT_REAL:
1851-
*std::get<double *>(m_value) = m_decl.GetDefault<double>();
1852-
break;
1853-
case GAAT_STRING_LIST:
1854-
*std::get<std::vector<std::string> *>(m_value) =
1855-
m_decl.GetDefault<std::vector<std::string>>();
1856-
break;
1857-
case GAAT_INTEGER_LIST:
1858-
*std::get<std::vector<int> *>(m_value) =
1859-
m_decl.GetDefault<std::vector<int>>();
1860-
break;
1861-
case GAAT_REAL_LIST:
1862-
*std::get<std::vector<double> *>(m_value) =
1863-
m_decl.GetDefault<std::vector<double>>();
1864-
break;
1865-
case GAAT_DATASET:
1866-
case GAAT_DATASET_LIST:
1867-
break;
1894+
switch (m_decl.GetType())
1895+
{
1896+
case GAAT_BOOLEAN:
1897+
*std::get<bool *>(m_value) = m_decl.GetDefault<bool>();
1898+
break;
1899+
case GAAT_STRING:
1900+
*std::get<std::string *>(m_value) =
1901+
m_decl.GetDefault<std::string>();
1902+
break;
1903+
case GAAT_INTEGER:
1904+
*std::get<int *>(m_value) = m_decl.GetDefault<int>();
1905+
break;
1906+
case GAAT_REAL:
1907+
*std::get<double *>(m_value) =
1908+
m_decl.GetDefault<double>();
1909+
break;
1910+
case GAAT_STRING_LIST:
1911+
*std::get<std::vector<std::string> *>(m_value) =
1912+
m_decl.GetDefault<std::vector<std::string>>();
1913+
break;
1914+
case GAAT_INTEGER_LIST:
1915+
*std::get<std::vector<int> *>(m_value) =
1916+
m_decl.GetDefault<std::vector<int>>();
1917+
break;
1918+
case GAAT_REAL_LIST:
1919+
*std::get<std::vector<double> *>(m_value) =
1920+
m_decl.GetDefault<std::vector<double>>();
1921+
break;
1922+
case GAAT_DATASET:
1923+
case GAAT_DATASET_LIST:
1924+
break;
1925+
}
1926+
}
1927+
catch (const std::bad_variant_access &)
1928+
{
1929+
// I don't think that can happen, but Coverity Scan thinks so
1930+
CPLError(CE_Failure, CPLE_AppDefined,
1931+
"Argument %s: SetDefault(): unexpected type for value",
1932+
GetName().c_str());
18681933
}
18691934
}
18701935
return *this;

0 commit comments

Comments
 (0)