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

321 diff mbox series

And turn it into an OptionsParser object.

Signed-off-by: Laurent Pinchart <laurent.pinchart@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

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@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@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel