From patchwork Tue Jan 22 03:03:54 2019
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
X-Patchwork-Submitter: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
X-Patchwork-Id: 321
Return-Path: <laurent.pinchart@ideasonboard.com>
Received: from perceval.ideasonboard.com (perceval.ideasonboard.com
	[213.167.242.64])
	by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7767B60B2D
	for <libcamera-devel@lists.libcamera.org>;
	Tue, 22 Jan 2019 04:03:59 +0100 (CET)
Received: from pendragon.bb.dnainternet.fi
	(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi
	[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])
	by perceval.ideasonboard.com (Postfix) with ESMTPSA id DFFE453E
	for <libcamera-devel@lists.libcamera.org>;
	Tue, 22 Jan 2019 04:03:58 +0100 (CET)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;
	s=mail; t=1548126239;
	bh=UlTyjp7izPD42PX99ZYBFaC/KzXEx7UNdaaRPEh2xxk=;
	h=From:To:Subject:Date:In-Reply-To:References:From;
	b=u7f3pKExpgUIdxIFISRCC8AUBRPiWf5Z3XXnXBe/z7bMjJ3Ka+DMKQKzwOB6TXPgP
	sbYgNAPMDgzytsalP0VJxS2aEwdNZmkOpZ9z1ZMbGoGdc8MVToG3a4tL+mYqb83JWr
	wuDVAiAvOklKqjgkZm+5ndJD5vPgt3iUs9CAkBPs=
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: libcamera-devel@lists.libcamera.org
Date: Tue, 22 Jan 2019 05:03:54 +0200
Message-Id: <20190122030354.9131-1-laurent.pinchart@ideasonboard.com>
X-Mailer: git-send-email 2.19.2
In-Reply-To: <20190121153331.19430-1-niklas.soderlund@ragnatech.se>
References: <20190121153331.19430-1-niklas.soderlund@ragnatech.se>
MIME-Version: 1.0
Subject: [libcamera-devel] [PATCH] cam: Extract option parser to separate
	file
X-BeenThere: libcamera-devel@lists.libcamera.org
X-Mailman-Version: 2.1.23
Precedence: list
List-Id: <libcamera-devel.lists.libcamera.org>
List-Unsubscribe: <https://lists.libcamera.org/options/libcamera-devel>,
	<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>
List-Archive: <https://lists.libcamera.org/pipermail/libcamera-devel/>
List-Post: <mailto:libcamera-devel@lists.libcamera.org>
List-Help: <mailto:libcamera-devel-request@lists.libcamera.org?subject=help>
List-Subscribe: <https://lists.libcamera.org/listinfo/libcamera-devel>,
	<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>
X-List-Received-Date: Tue, 22 Jan 2019 03:03:59 -0000

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__ */