[RFC PATCH v1 06/12] apps: lc-compliance: Use `std::vector` for argument array

[Barnabás Pőcze]

Hi


2025. január 10., péntek 1:21 keltezéssel, Laurent Pinchart <laurent.pinchart at ideasonboard.com> írta:

> Hi Barnabás,
> 
> Thank you for the patch.
> 
> On Fri, Dec 20, 2024 at 03:08:29PM +0000, Barnabás Pőcze wrote:
> > Just use an `std::vector` to store the arguments passed to
> > `InitGoogleTest()`. This removes the need for the map and
> > the separate `argc` variable used for size-keeping.
> >
> > Signed-off-by: Barnabás Pőcze <pobrn at protonmail.com>
> > ---
> >  src/apps/lc-compliance/main.cpp | 36 +++++++++------------------------
> >  1 file changed, 9 insertions(+), 27 deletions(-)
> >
> > diff --git a/src/apps/lc-compliance/main.cpp b/src/apps/lc-compliance/main.cpp
> > index cdd0bd515..3d9c51fc3 100644
> > --- a/src/apps/lc-compliance/main.cpp
> > +++ b/src/apps/lc-compliance/main.cpp
> > @@ -80,45 +80,27 @@ static int initCamera(CameraManager *cm, OptionsParser::Options options)
> >
> >  static int initGtestParameters(char *arg0, OptionsParser::Options options)
> >  {
> > -	const std::map<std::string, std::string> gtestFlags = { { "list", "--gtest_list_tests" },
> > -								{ "filter", "--gtest_filter" } };
> > -
> > -	int argc = 0;
> > +	std::vector<char *> argv;
> >  	std::string filterParam;
> >
> > -	/*
> > -	 * +2 to have space for both the 0th argument that is needed but not
> > -	 * used and the null at the end.
> > -	 */
> > -	char **argv = new char *[(gtestFlags.size() + 2)];
> > -	if (!argv)
> > -		return -ENOMEM;
> > -
> > -	argv[0] = arg0;
> > -	argc++;
> > +	argv.push_back(arg0);
> >
> > -	if (options.isSet(OptList)) {
> > -		argv[argc] = const_cast<char *>(gtestFlags.at("list").c_str());
> > -		argc++;
> > -	}
> > +	if (options.isSet(OptList))
> > +		argv.push_back(const_cast<char *>("--gtest_list_tests"));
> 
> Functions that take a char ** when they never modify the contents of the
> strings are annoying :-/
> 
> Would it be better to drop the const_char here and below, and add a
> single one when calling ::testing::InitGoogleTest() with
> 
> 	::testing::InitGoogleTest(&argc, const_cast<char **>(argv.data()));
> 
> ?

Yes, that works, so I'll move the `const_cast` into the `InitGoogleTest()` invocation.


Regards,
Barnabás Pőcze


> 
> Either way,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> >
> >  	if (options.isSet(OptFilter)) {
> >  		/*
> >  		 * The filter flag needs to be passed as a single parameter, in
> >  		 * the format --gtest_filter=filterStr
> >  		 */
> > -		filterParam = gtestFlags.at("filter") + "=" +
> > -			      static_cast<const std::string &>(options[OptFilter]);
> > -
> > -		argv[argc] = const_cast<char *>(filterParam.c_str());
> > -		argc++;
> > +		filterParam = "--gtest_filter=" + options[OptFilter].toString();
> > +		argv.push_back(const_cast<char *>(filterParam.c_str()));
> >  	}
> >
> > -	argv[argc] = nullptr;
> > -
> > -	::testing::InitGoogleTest(&argc, argv);
> > +	argv.push_back(nullptr);
> >
> > -	delete[] argv;
> > +	int argc = argv.size();
> > +	::testing::InitGoogleTest(&argc, argv.data());
> >
> >  	return 0;
> >  }
> 
> --
> Regards,
> 
> Laurent Pinchart
>