Fix fiptool packaging issue on windows
Windows does not have a standard getopt implementation. To address
this an equivalent implementation has been provided in win_posix.c
However, the implementation has an issue with option processing as
described below.
Long option names may be abbreviated if the abbreviation is unique
or an exact match for some defined option.
Since some options can be substring of other options e.g. "scp-fw"
option is a substring of "scp-fwu-cfg", we need to identify if an
option is abbreviated and also check for uniqueness. Otherwise if
a user passes --scp-fw as an option, the "scp-fwu-cfg" option may
get selected, resulting in an incorrectly packaged FIP.
This issue has been be fixed by:
- First searching for an exact match.
- If exact match was not found search for a abbreviated match.
By doing this an incorrect option selection can be avoided.
Change-Id: I22f4e7a683f3df857f5b6f0783bf9b03a64a0bcc
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
diff --git a/tools/fiptool/win_posix.c b/tools/fiptool/win_posix.c
index 48feb16..33b44d4 100644
--- a/tools/fiptool/win_posix.c
+++ b/tools/fiptool/win_posix.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2017, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2017 - 2020, Arm Limited and Contributors. All rights reserved.
*
* SPDX-License-Identifier: BSD-3-Clause
*/
@@ -137,7 +137,8 @@
* Note that we only match over the shorter length of the pair, to allow
* for abbreviation or say --match=value
* Long option names may be abbreviated if the abbreviation is unique or an
- * exact match for some defined option.
+ * exact match for some defined option. This function does not check that the
+ * abbreviations are unique and should be handled by the caller.
* A long option may take a parameter, of the form --opt=param or --opt param.
*/
static
@@ -160,42 +161,72 @@
{
int result = RET_UNKNOWN_OPT;
size_t loptn = 0;
+ bool match_found = false;
- while (longopts[loptn].name != 0) {
- if (optmatch(optname, longopts[loptn].name) == 0) {
- /* We found a match. */
- result = longopts[loptn].val;
- if (indexptr != 0)
- *indexptr = loptn;
- switch (longopts[loptn].has_arg) {
- case required_argument:
- if ((optind + 1) >= argc) {
- /* Missing argument. */
- optopt = result;
- return RET_NO_PARAM;
- }
- /* Fallthrough to get option value. */
+ /*
+ * Long option names may be abbreviated if the abbreviation
+ * is unique or an exact match for some defined option.
+ * To handle this:
+ * - First search for an exact match.
+ * - If exact match was not found search for a abbreviated match.
+ * By doing this an incorrect option selection can be avoided.
+ */
- case optional_argument:
- if ((argc - optind) > 0) {
- /* Found argument. */
- optarg = argv[++optind];
- }
- /* Fallthrough to handle flag. */
+ /* 1. Search for an exact match. */
+ while (longopts[loptn].name != NULL) {
+ if (strcmp(optname, longopts[loptn].name) == 0) {
+ match_found = true;
+ break;
+ }
+ ++loptn;
+ }
- case no_argument:
- optind++;
- if (longopts[loptn].flag != 0) {
- *longopts[loptn].flag = result;
- result = 0;
- }
+ /* 2. If exact match was not found search for a abbreviated match. */
+ if (!match_found) {
+ loptn = 0;
+ while (longopts[loptn].name != NULL) {
+ if (optmatch(optname, longopts[loptn].name) == 0) {
+ match_found = true;
break;
+ }
+ ++loptn;
+ }
+ }
+
+ if (match_found) {
+ /* We found a match. */
+ result = longopts[loptn].val;
+ if (indexptr != 0) {
+ *indexptr = loptn;
+ }
+ switch (longopts[loptn].has_arg) {
+ case required_argument:
+ if ((optind + 1) >= argc) {
+ /* Missing argument. */
+ optopt = result;
+ return RET_NO_PARAM;
+ }
+ /* Fallthrough to get option value. */
+ case optional_argument:
+ if ((argc - optind) > 0) {
+ /* Found argument. */
+ optarg = argv[++optind];
}
- return result;
+ /* Fallthrough to handle flag. */
+
+ case no_argument:
+ optind++;
+ if (longopts[loptn].flag != 0) {
+ *longopts[loptn].flag = result;
+ result = 0;
+ }
+ break;
+
}
- ++loptn;
+ return result;
}
+
/*
* If getopt finds an option character in argv that was not included
* in options, ... it returns '?' and sets the external variable
diff --git a/tools/fiptool/win_posix.h b/tools/fiptool/win_posix.h
index 836ffed..6f0d8e6 100644
--- a/tools/fiptool/win_posix.h
+++ b/tools/fiptool/win_posix.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2017-2018, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2017-2020, Arm Limited and Contributors. All rights reserved.
*
* SPDX-License-Identifier: BSD-3-Clause
*/
@@ -9,13 +9,15 @@
#define _CRT_SECURE_NO_WARNINGS
-#include <direct.h>
-#include <io.h>
+#include <stdbool.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>
+#include <direct.h>
+#include <io.h>
+
#include "uuid.h"
/* Derive or provide Windows equivalents of Posix/GCC/Unix stuff. */