From cb5c08b6980bddc9dbe4c825da3914e4ae38a113 Mon Sep 17 00:00:00 2001 From: Sami Mujawar Date: Thu, 30 Apr 2020 12:40:22 +0100 Subject: [PATCH 1/2] 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 --- tools/fiptool/win_posix.c | 97 ++++++++++++++++++++++++++------------- tools/fiptool/win_posix.h | 8 ++-- 2 files changed, 69 insertions(+), 36 deletions(-) diff --git a/tools/fiptool/win_posix.c b/tools/fiptool/win_posix.c index 48feb162e..33b44d4c6 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 @@ int getopt(int argc, * 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 getopt_1long(const int argc, { 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. */ - - case no_argument: - optind++; - if (longopts[loptn].flag != 0) { - *longopts[loptn].flag = result; - result = 0; - } - break; - - } - return result; + /* 1. Search for an exact match. */ + while (longopts[loptn].name != NULL) { + if (strcmp(optname, longopts[loptn].name) == 0) { + match_found = true; + break; } ++loptn; } + + /* 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]; + } + /* Fallthrough to handle flag. */ + + case no_argument: + optind++; + if (longopts[loptn].flag != 0) { + *longopts[loptn].flag = result; + result = 0; + } + break; + + } + 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 836ffed31..6f0d8e6b6 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 -#include +#include #include #include #include #include +#include +#include + #include "uuid.h" /* Derive or provide Windows equivalents of Posix/GCC/Unix stuff. */ From 88a1cf1e4e8032580058745ba221abc5d8b90ef3 Mon Sep 17 00:00:00 2001 From: Sami Mujawar Date: Thu, 30 Apr 2020 12:41:57 +0100 Subject: [PATCH 2/2] Update makefile to build fiptool for Windows Although support for building fiptool on a Windows host was present, the binary was not built when the top level makefile was invoked. This patch makes the necessary changes to the to support building of fiptool on a Windows host PC from the main makefile. Change-Id: I0c01ba237fa3010a027a1b324201131210cf4d7c Signed-off-by: Sami Mujawar --- Makefile | 21 ++++++++++++ make_helpers/windows.mk | 4 ++- tools/fiptool/Makefile.msvc | 67 ++++++++++++++++++++----------------- 3 files changed, 61 insertions(+), 31 deletions(-) diff --git a/Makefile b/Makefile index af829f2bd..6797c4d20 100644 --- a/Makefile +++ b/Makefile @@ -1157,7 +1157,13 @@ endif clean: @echo " CLEAN" $(call SHELL_REMOVE_DIR,${BUILD_PLAT}) +ifdef UNIX_MK ${Q}${MAKE} --no-print-directory -C ${FIPTOOLPATH} clean +else +# Clear the MAKEFLAGS as we do not want +# to pass the gnumake flags to nmake. + ${Q}set MAKEFLAGS= && ${MSVC_NMAKE} /nologo /f ${FIPTOOLPATH}/Makefile.msvc FIPTOOLPATH=$(subst /,\,$(FIPTOOLPATH)) FIPTOOL=$(subst /,\,$(FIPTOOL)) clean +endif ${Q}${MAKE} PLAT=${PLAT} --no-print-directory -C ${CRTTOOLPATH} clean ${Q}${MAKE} PLAT=${PLAT} --no-print-directory -C ${ENCTOOLPATH} clean ${Q}${MAKE} --no-print-directory -C ${ROMLIBPATH} clean @@ -1166,7 +1172,13 @@ realclean distclean: @echo " REALCLEAN" $(call SHELL_REMOVE_DIR,${BUILD_BASE}) $(call SHELL_DELETE_ALL, ${CURDIR}/cscope.*) +ifdef UNIX_MK ${Q}${MAKE} --no-print-directory -C ${FIPTOOLPATH} clean +else +# Clear the MAKEFLAGS as we do not want +# to pass the gnumake flags to nmake. + ${Q}set MAKEFLAGS= && ${MSVC_NMAKE} /nologo /f ${FIPTOOLPATH}/Makefile.msvc FIPTOOLPATH=$(subst /,\,$(FIPTOOLPATH)) FIPTOOL=$(subst /,\,$(FIPTOOL)) realclean +endif ${Q}${MAKE} --no-print-directory -C ${SPTOOLPATH} clean ${Q}${MAKE} PLAT=${PLAT} --no-print-directory -C ${CRTTOOLPATH} clean ${Q}${MAKE} PLAT=${PLAT} --no-print-directory -C ${ENCTOOLPATH} realclean @@ -1252,7 +1264,16 @@ fwu_fip: ${BUILD_PLAT}/${FWU_FIP_NAME} .PHONY: ${FIPTOOL} ${FIPTOOL}: + @${ECHO_BLANK_LINE} + @echo "Building $@" +ifdef UNIX_MK ${Q}${MAKE} CPPFLAGS="-DVERSION='\"${VERSION_STRING}\"'" FIPTOOL=${FIPTOOL} --no-print-directory -C ${FIPTOOLPATH} +else +# Clear the MAKEFLAGS as we do not want +# to pass the gnumake flags to nmake. + ${Q}set MAKEFLAGS= && ${MSVC_NMAKE} /nologo /f ${FIPTOOLPATH}/Makefile.msvc FIPTOOLPATH=$(subst /,\,$(FIPTOOLPATH)) FIPTOOL=$(subst /,\,$(FIPTOOL)) +endif + @${ECHO_BLANK_LINE} sptool: ${SPTOOL} .PHONY: ${SPTOOL} diff --git a/make_helpers/windows.mk b/make_helpers/windows.mk index 5ab8bdc4f..26ea88ef0 100644 --- a/make_helpers/windows.mk +++ b/make_helpers/windows.mk @@ -1,5 +1,5 @@ # -# Copyright (c) 2016-2018, ARM Limited and Contributors. All rights reserved. +# Copyright (c) 2016-2020, ARM Limited and Contributors. All rights reserved. # # SPDX-License-Identifier: BSD-3-Clause # @@ -86,3 +86,5 @@ define MAKE_BUILD_STRINGS $$(CC) $$(TF_CFLAGS) $$(CFLAGS) -x c -c - -o $1 endef +MSVC_NMAKE := nmake.exe + diff --git a/tools/fiptool/Makefile.msvc b/tools/fiptool/Makefile.msvc index 58dbb8973..9081bc64c 100644 --- a/tools/fiptool/Makefile.msvc +++ b/tools/fiptool/Makefile.msvc @@ -1,30 +1,37 @@ -# -# Copyright (c) 2019, Arm Limited. All rights reserved. -# -# SPDX-License-Identifier: BSD-3-Clause -# - -CC = cl.exe -LD = link.exe - -FIPTOOL = fiptool.exe -OBJECTS = fiptool.obj tbbr_config.obj win_posix.obj - -INC = -I. -I..\..\include\tools_share -CFLAGS = $(CFLAGS) /nologo /Za /Zi /c /O2 /MT - -all: $(FIPTOOL) - -$(FIPTOOL): $(OBJECTS) - $(LD) /INCREMENTAL:NO /debug /nodefaultlib:libc.lib /out:$@ $(LIBS) $** - -.PHONY: clean realclean - -clean: - del /f /q $(OBJECTS) > nul - -realclean: - del /f /q $(OBJECTS) $(FIPTOOL) > nul - -.c.obj: - $(CC) -c $(CFLAGS) $(INC) $< -Fo$@ +# +# Copyright (c) 2019-2020, Arm Limited. All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause +# + +CC = cl.exe +LD = link.exe + +# FIPTOOLPATH and FIPTOOL are passed from the main makefile. + +OBJECTS = $(FIPTOOLPATH)\fiptool.obj \ + $(FIPTOOLPATH)\tbbr_config.obj \ + $(FIPTOOLPATH)\win_posix.obj + +INC = -I$(FIPTOOLPATH) -Iinclude\tools_share + +CFLAGS = $(CFLAGS) /nologo /Za /Zi /c /O2 /MT + +all: $(FIPTOOL) + +$(FIPTOOL): $(OBJECTS) + $(LD) /nologo /INCREMENTAL:NO /debug /nodefaultlib:libc.lib /out:$@ $(LIBS) $** + +.PHONY: clean realclean + +clean: + -@del /f /q $(OBJECTS) > nul + -@del /f /q $(FIPTOOLPATH)\*.pdb > nul + +realclean: + -@del /f /q $(OBJECTS) > nul + -@del /f /q $(FIPTOOLPATH)\*.pdb > nul + -@del /f /q $(FIPTOOL) > nul + +.c.obj: + $(CC) -c $(CFLAGS) $(INC) $< -Fo$@