[libcamera-devel] [PATCH] cam: Extract option parser to separate file

Niklas Söderlund niklas.soderlund at ragnatech.se
Tue Jan 22 14:07:53 CET 2019


Hello,

These two patches have been merged.

On 2019-01-22 05:03:54 +0200, Laurent Pinchart wrote:
> And turn it into an OptionsParser object.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> 
> Hi Niklas,
> 
> On top of your cam utility patch, a bit of cleanup and argument parsing
> refactoring. With this on top the base patch is good for me, feel free
> to push the combination (in which case please don't forget to add your
> SoB to this patch) - possibly after waiting for more review.
> 
>  src/cam/{cam.cpp => main.cpp} | 117 +++++----------------
>  src/cam/meson.build           |   3 +-
>  src/cam/options.cpp           | 192 ++++++++++++++++++++++++++++++++++
>  src/cam/options.h             |  62 +++++++++++
>  4 files changed, 285 insertions(+), 89 deletions(-)
>  rename src/cam/{cam.cpp => main.cpp} (22%)
>  create mode 100644 src/cam/options.cpp
>  create mode 100644 src/cam/options.h
> 
> diff --git a/src/cam/cam.cpp b/src/cam/main.cpp
> similarity index 22%
> rename from src/cam/cam.cpp
> rename to src/cam/main.cpp
> index 0f795be78106..22211670c625 100644
> --- a/src/cam/cam.cpp
> +++ b/src/cam/main.cpp
> @@ -2,139 +2,80 @@
>  /*
>   * Copyright (C) 2019, Google Inc.
>   *
> - * main.cpp - cam-ctl a tool to interact with the library
> + * main.cpp - cam - The libcamera swiss army knife
>   */
>  
> -#include <getopt.h>
> -#include <iomanip>
>  #include <iostream>
>  #include <map>
>  #include <string.h>
>  
>  #include <libcamera/libcamera.h>
>  
> -#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
> +#include "options.h"
>  
> -using namespace std;
>  using namespace libcamera;
>  
> -enum Option {
> +OptionsParser::Options options;
> +
> +enum {
>  	OptCamera = 'c',
>  	OptHelp = 'h',
>  	OptList = 'l',
> -	OptLast = 0,
> -};
> -
> -struct OptionInfo {
> -	Option id;
> -	const char *name;
> -	const char *arguments;
> -	const char *description;
>  };
>  
> -static struct OptionInfo option_info[] = {
> -	{ OptCamera, "camera", "<camera>", "Specify which camera to operate on" },
> -	{ OptHelp, "help", nullptr, "Display this help message" },
> -	{ OptList, "list", nullptr, "List all cameras" },
> -	{ OptLast, nullptr, nullptr, nullptr },
> -};
> -
> -std::map<Option, std::string> options;
> -
> -void usage()
> +static int parseOptions(int argc, char *argv[])
>  {
> -	struct OptionInfo *info;
> +	OptionsParser parser;
>  
> -	cout << "Options:" << endl;
> -	for (info = option_info; info->id != OptLast; info++) {
> -		string arg(info->name);
> +	parser.addOption(OptCamera, "Specify which camera to operate on",
> +			 "camera", OptionsParser::ArgumentRequired,
> +			 "camera");
> +	parser.addOption(OptHelp, "Display this help message", "help");
> +	parser.addOption(OptList, "List all cameras", "list");
>  
> -		if (info->arguments)
> -			arg += string(" ") + info->arguments;
> +	options = std::move(parser.parse(argc, argv));
> +	if (!options.valid())
> +		return -EINVAL;
>  
> -		cout << "  -" << static_cast<char>(info->id) << " --" <<
> -			setw(20) << left << arg << " - " <<
> -			info->description << endl;
> -	}
> -}
> -
> -int parseOptions(int argc, char **argv)
> -{
> -	char short_options[ARRAY_SIZE(option_info) * 2 + 1];
> -	struct option long_options[ARRAY_SIZE(option_info)];
> -	struct OptionInfo *info;
> -	unsigned ids = 0, idl = 0;
> -
> -	memset(short_options, 0, sizeof(short_options));
> -	memset(long_options, 0, sizeof(long_options));
> -
> -	for (info = option_info; info->id != OptLast; info++) {
> -		short_options[ids++] = info->id;
> -		if (info->arguments)
> -			short_options[ids++] = ':';
> -
> -		long_options[idl].name = info->name;
> -		long_options[idl].has_arg =
> -			info->arguments ? required_argument : no_argument;
> -		long_options[idl].flag = 0;
> -		long_options[idl].val = info->id;
> -		idl++;
> -	}
> -
> -	while (true) {
> -		int c = getopt_long(argc, argv, short_options, long_options, nullptr);
> -
> -		if (c == -1)
> -			break;
> -
> -		if (!isalpha(c))
> -			return EXIT_FAILURE;
> -
> -		options[static_cast<Option>(c)] = optarg ? string(optarg) : "";
> +	if (argc == 1 || options.isSet(OptHelp)) {
> +		parser.usage();
> +		return 1;
>  	}
>  
>  	return 0;
>  }
>  
> -bool optSet(Option opt)
> -{
> -	return options.count(opt) != 0;
> -}
> -
>  int main(int argc, char **argv)
>  {
>  	int ret;
>  
>  	ret = parseOptions(argc, argv);
> -	if (ret == EXIT_FAILURE)
> -		return ret;
> -
> -	if (argc == 1 || optSet(OptHelp)) {
> -		usage();
> -		return 0;
> -	}
> +	if (ret < 0)
> +		return EXIT_FAILURE;
>  
>  	CameraManager *cm = CameraManager::instance();
>  
>  	ret = cm->start();
>  	if (ret) {
> -		cout << "Failed to start camera manager: " << strerror(-ret) << endl;
> +		std::cout << "Failed to start camera manager: "
> +			  << strerror(-ret) << std::endl;
>  		return EXIT_FAILURE;
>  	}
>  
> -	if (optSet(OptList)) {
> -		cout << "Available cameras:" << endl;
> +	if (options.isSet(OptList)) {
> +		std::cout << "Available cameras:" << std::endl;
>  		for (const std::shared_ptr<Camera> &camera : cm->cameras())
> -			cout << "- " << camera->name() << endl;
> +			std::cout << "- " << camera->name() << std::endl;
>  	}
>  
> -	if (optSet(OptCamera)) {
> +	if (options.isSet(OptCamera)) {
>  		std::shared_ptr<Camera> cam = cm->get(options[OptCamera]);
>  
>  		if (cam) {
> -			cout << "Using camera " << cam->name() << endl;
> +			std::cout << "Using camera " << cam->name() << std::endl;
>  		} else {
> -			cout << "Camera " << options[OptCamera] << " not found" << endl;
> +			std::cout << "Camera " << options[OptCamera]
> +				  << " not found" << std::endl;
>  		}
>  	}
>  
> diff --git a/src/cam/meson.build b/src/cam/meson.build
> index 809a40e03492..e45e5391f679 100644
> --- a/src/cam/meson.build
> +++ b/src/cam/meson.build
> @@ -1,5 +1,6 @@
>  cam_sources = files([
> -    'cam.cpp',
> +    'main.cpp',
> +    'options.cpp',
>  ])
>  
>  cam  = executable('cam', cam_sources,
> diff --git a/src/cam/options.cpp b/src/cam/options.cpp
> new file mode 100644
> index 000000000000..d391a0e58436
> --- /dev/null
> +++ b/src/cam/options.cpp
> @@ -0,0 +1,192 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * options.cpp - cam - Options parsing
> + */
> +
> +#include <getopt.h>
> +#include <iomanip>
> +#include <iostream>
> +#include <string.h>
> +
> +#include "options.h"
> +
> +void OptionsParser::addOption(int opt, const char *help, const char *name,
> +			      OptionArgument argument, const char *argumentName)
> +{
> +	/*
> +	 * Options must have at least a short or long name, and a text message.
> +	 * If an argument is accepted, it must be described by argumentName.
> +	 */
> +	if (!isalnum(opt) && !name)
> +		return;
> +	if (!help || help[0] == '\0')
> +		return;
> +	if (argument != ArgumentNone && !argumentName)
> +		return;
> +
> +	/* Reject duplicate options. */
> +	if (optionsMap_.find(opt) != optionsMap_.end())
> +		return;
> +
> +	options_.push_back(Option({ opt, name, argument, argumentName, help }));
> +	optionsMap_[opt] = &options_.back();
> +}
> +
> +OptionsParser::Options OptionsParser::parse(int argc, char **argv)
> +{
> +	OptionsParser::Options options;
> +
> +	/*
> +	 * Allocate short and long options arrays large enough to contain all
> +	 * options.
> +	 */
> +	char shortOptions[options_.size() * 3 + 2] = {};
> +	struct option longOptions[options_.size() + 1] = {};
> +	unsigned int ids = 0;
> +	unsigned int idl = 0;
> +
> +	shortOptions[ids++] = ':';
> +
> +	for (const Option &option : options_) {
> +		if (option.hasShortOption()) {
> +			shortOptions[ids++] = option.opt;
> +			if (option.argument != ArgumentNone)
> +				shortOptions[ids++] = ':';
> +			if (option.argument == ArgumentOptional)
> +				shortOptions[ids++] = ':';
> +		}
> +
> +		if (option.hasLongOption()) {
> +			longOptions[idl].name = option.name;
> +
> +			switch (option.argument) {
> +			case ArgumentNone:
> +				longOptions[idl].has_arg = no_argument;
> +				break;
> +			case ArgumentRequired:
> +				longOptions[idl].has_arg = required_argument;
> +				break;
> +			case ArgumentOptional:
> +				longOptions[idl].has_arg = optional_argument;
> +				break;
> +			}
> +
> +			longOptions[idl].flag = 0;
> +			longOptions[idl].val = option.opt;
> +			idl++;
> +		}
> +	}
> +
> +	opterr = 0;
> +
> +	while (true) {
> +		int c = getopt_long(argc, argv, shortOptions, longOptions, nullptr);
> +
> +		if (c == -1)
> +			break;
> +
> +		if (c == '?' || c == ':') {
> +			if (c == '?')
> +				std::cerr << "Invalid option ";
> +			else
> +				std::cerr << "Missing argument for option ";
> +			std::cerr << argv[optind - 1] << std::endl;
> +
> +			usage();
> +			options.clear();
> +			break;
> +		}
> +
> +		options.values_[c] = optarg ? optarg : "";
> +	}
> +
> +	return std::move(options);
> +}
> +
> +void OptionsParser::usage()
> +{
> +	std::cerr << "Options:" << std::endl;
> +
> +	unsigned int indent = 0;
> +
> +	for (const Option &option : options_) {
> +		unsigned int length = 14;
> +		if (option.hasLongOption())
> +			length += 2 + strlen(option.name);
> +		if (option.argument != ArgumentNone)
> +			length += 1 + strlen(option.argumentName);
> +		if (option.argument == ArgumentOptional)
> +			length += 2;
> +
> +		if (length > indent)
> +			indent = length;
> +	}
> +
> +	indent = (indent + 7) / 8 * 8;
> +
> +	for (const Option &option : options_) {
> +		std::string argument;
> +		if (option.hasShortOption())
> +			argument = std::string("  -")
> +				 + static_cast<char>(option.opt);
> +		else
> +			argument = "    ";
> +
> +		if (option.hasLongOption()) {
> +			if (option.hasShortOption())
> +				argument += ", ";
> +			else
> +				argument += "  ";
> +			argument += std::string("--") + option.name;
> +		};
> +
> +		if (option.argument != ArgumentNone) {
> +			argument += std::string(" ");
> +			if (option.argument == ArgumentOptional)
> +				argument += "[";
> +			argument += option.argumentName;
> +			if (option.argument == ArgumentOptional)
> +				argument += "]";
> +		}
> +
> +		std::cerr << std::setw(indent) << std::left << argument;
> +		std::cerr << option.help << std::endl;
> +	}
> +}
> +
> +OptionsParser::Options::Options()
> +{
> +}
> +
> +OptionsParser::Options::Options(Options &&other)
> +	: values_(std::move(other.values_))
> +{
> +}
> +
> +OptionsParser::Options &OptionsParser::Options::operator=(Options &&other)
> +{
> +	values_ = other.values_;
> +	return *this;
> +}
> +
> +bool OptionsParser::Options::valid() const
> +{
> +	return !values_.empty();
> +}
> +
> +bool OptionsParser::Options::isSet(int opt) const
> +{
> +	return values_.find(opt) != values_.end();
> +}
> +
> +const std::string &OptionsParser::Options::operator[](int opt) const
> +{
> +	return values_.find(opt)->second;
> +}
> +
> +void OptionsParser::Options::clear()
> +{
> +	values_.clear();
> +}
> diff --git a/src/cam/options.h b/src/cam/options.h
> new file mode 100644
> index 000000000000..88336dfe3cc6
> --- /dev/null
> +++ b/src/cam/options.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * options.h - cam - Options parsing
> + */
> +#ifndef __CAM_OPTIONS_H__
> +#define __CAM_OPTIONS_H__
> +
> +#include <ctype.h>
> +#include <map>
> +#include <vector>
> +
> +class OptionsParser
> +{
> +public:
> +	enum OptionArgument {
> +		ArgumentNone,
> +		ArgumentRequired,
> +		ArgumentOptional,
> +	};
> +
> +	class Options {
> +	public:
> +		Options();
> +		Options(Options &&other);
> +		Options &operator=(Options &&other);
> +
> +		bool valid() const;
> +		bool isSet(int opt) const;
> +		const std::string &operator[](int opt) const;
> +
> +	private:
> +		friend class OptionsParser;
> +		std::map<int, std::string> values_;
> +		void clear();
> +	};
> +
> +	void addOption(int opt, const char *help, const char *name = nullptr,
> +		       OptionArgument argument = ArgumentNone,
> +		       const char *argumentName = nullptr);
> +
> +	Options parse(int argc, char *argv[]);
> +	void usage();
> +
> +private:
> +	struct Option {
> +		int opt;
> +		const char *name;
> +		OptionArgument argument;
> +		const char *argumentName;
> +		const char *help;
> +
> +		bool hasShortOption() const { return isalnum(opt); }
> +		bool hasLongOption() const { return name != nullptr; }
> +	};
> +
> +	std::vector<Option> options_;
> +	std::map<unsigned int, Option *> optionsMap_;
> +};
> +
> +#endif /* __CAM_OPTIONS_H__ */
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund


More information about the libcamera-devel mailing list