Merge pull request #1186 from en-sc/en-sc/from_upstream

Merge up to 133dd9d from  upstream

Author: en-sc

.gitignore

@@ -11,9 +11,9 @@
 *.la
 *.in
 
-# generated source files
-src/jtag/minidriver_imp.h
-src/jtag/jtag_minidriver.h
+# coverage files (gcov)
+*.gcda
+*.gcno
 
 # OpenULINK driver files generated by SDCC
 src/jtag/drivers/OpenULINK/*.rel

.gitmodules

@@ -1,6 +1,3 @@
-[submodule "tools/git2cl"]
-	path = tools/git2cl
-	url = https://git.savannah.nongnu.org/git/git2cl.git
 [submodule "jimtcl"]
 	path = jimtcl
 	url = https://github.com/msteveb/jimtcl.git

HACKING

@@ -118,6 +118,22 @@ patch:
   make
   @endcode
 
+- Code coverage analysis
+
+  By inspecting the code coverage, you can identify potential gaps in your testing
+  and use that information to improve your test scenarios.
+
+  Example usage:
+  @code
+  mkdir build-gcov; cd build-gcov
+  ../configure --enable-gcov [...]
+  make
+  # ... Now execute your test scenarios to collect OpenOCD code coverage ...
+  lcov --capture --directory ./src --output-file openocd-coverage.info
+  genhtml openocd-coverage.info --output-directory coverage_report
+  # ... Open coverage_report/index.html in a web browser ...
+  @endcode
+
 Please consider performing these additional checks where appropriate
 (especially Clang Static Analyzer for big portions of new code) and
 mention the results (e.g. "Valgrind-clean, no new Clang analyzer

Makefile.am

@@ -38,6 +38,7 @@ endif
 
 # common flags used in openocd build
 AM_CFLAGS = $(GCC_WARNINGS)
+AM_LDFLAGS =
 
 AM_CPPFLAGS = $(HOST_CPPFLAGS)\
 			  -I$(top_srcdir)/src \
@@ -51,6 +52,12 @@ AM_CPPFLAGS += -I$(top_srcdir)/jimtcl \
 else
 AM_CPPFLAGS += $(JIMTCL_CFLAGS)
 endif
+
+if USE_GCOV
+AM_CFLAGS += --coverage
+AM_LDFLAGS += --coverage
+endif
+
 EXTRA_DIST += \
 	BUGS \
 	HACKING \
@@ -116,14 +123,13 @@ TCL_PATH = tcl
 TCL_FILES = find $(srcdir)/$(TCL_PATH) -name '*.cfg' -o -name '*.tcl' -o -name '*.txt' | \
 		sed -e 's,^$(srcdir)/$(TCL_PATH),,'
 
-# Without the PERL_UNICODE="IO" workaround below when running git2cl, you get several
-# "Wide character" warnings and you also risk an invalid character encoding in
-# the generated ChangeLog file. For more information, see this bug report:
-#   Warning "Wide character in print"
-#   https://savannah.nongnu.org/bugs/?65689
+# The git log command below generates many empty text lines with only some space characters
+# for indentation purposes, so use sed to trim all trailing whitespace.
 dist-hook:
 	if test -d $(srcdir)/.git -a \( ! -e $(distdir)/ChangeLog -o -w $(distdir)/ChangeLog \) ; then \
-		git --git-dir $(srcdir)/.git log | PERL_UNICODE="IO" $(srcdir)/tools/git2cl/git2cl > $(distdir)/ChangeLog ; \
+		git --git-dir $(srcdir)/.git log --date=short --pretty="format:%ad  %aN <%aE>%n%n%w(0,4,6)* %B" \
+		| sed 's/[[:space:]]*$$//' > $(distdir)/ChangeLog.tmp && \
+		mv $(distdir)/ChangeLog.tmp $(distdir)/ChangeLog; \
 	fi
 	for i in $$($(TCL_FILES)); do \
 		j="$(distdir)/$(TCL_PATH)/$$i" && \

bootstrap

@@ -3,8 +3,8 @@
 
 # Run the autotools bootstrap sequence to create the configure script
 
-# Abort execution on error
-set -e
+set -e  # Abort execution on error.
+set -u  # Abort if you reference an undefined variable.
 
 if which libtoolize > /dev/null; then
     libtoolize="libtoolize"
@@ -15,13 +15,21 @@ else
     exit 1
 fi
 
-if [ "$1" = "nosubmodule" ]; then
-    SKIP_SUBMODULE=1
-elif [ -n "$1" ]; then
-    echo "$0: Illegal argument $1"
-    echo "USAGE: $0 [nosubmodule]"
-    exit 1
-fi
+SKIP_SUBMODULE=0
+
+case "$#" in
+    0) ;;
+    1) if [ "$1" = "nosubmodule" ]; then
+           SKIP_SUBMODULE=1
+       else
+           echo "$0: Illegal argument $1" >&2
+           echo "USAGE: $0 [nosubmodule]" >&2
+           exit 1
+       fi;;
+   *)  echo "$0: Wrong number of command-line arguments." >&2
+       echo "USAGE: $0 [nosubmodule]" >&2
+       exit 1;;
+esac
 
 # bootstrap the autotools
 (
@@ -34,7 +42,7 @@ autoheader --warnings=all
 automake --warnings=all --gnu --add-missing --copy
 )
 
-if [ -n "$SKIP_SUBMODULE" ]; then
+if [ "$SKIP_SUBMODULE" -ne 0 ]; then
     echo "Skipping submodule setup"
 else
     echo "Setting up submodules"

configure.ac

@@ -172,6 +172,9 @@ m4_define([DUMMY_ADAPTER],
 m4_define([OPTIONAL_LIBRARIES],
 	[[[capstone], [Use Capstone disassembly framework], []]])
 
+m4_define([COVERAGE],
+	[[[gcov], [Collect coverage using gcov], []]])
+
 AC_ARG_ENABLE([doxygen-html],
   AS_HELP_STRING([--disable-doxygen-html],
     [Disable building Doxygen manual as HTML.]),
@@ -200,6 +203,19 @@ AC_ARG_ENABLE([werror],
   AS_HELP_STRING([--disable-werror], [Do not treat warnings as errors]),
   [gcc_werror=$enableval], [gcc_werror=$gcc_warnings])
 
+AC_ARG_ENABLE([gcov],
+  AS_HELP_STRING([--enable-gcov], [Enable runtime coverage collection via gcov]),
+  [enable_gcov=$enableval], [enable_gcov=no])
+
+AS_IF([test "x$enable_gcov" = "xyes"], [
+  AC_DEFINE([USE_GCOV], [1], [1 to enable coverage collection using gcov.])
+  dnl When collecting coverage, disable optimizations.
+  dnl This overrides the "-O2" that autoconf uses by default:
+  CFLAGS+=" -O0"
+], [
+  AC_DEFINE([USE_GCOV], [0], [0 to leave coverage collection disabled.])
+])
+
 # set default verbose options, overridden by following options
 debug_usb_io=no
 debug_usb_comms=no
@@ -257,9 +273,13 @@ AC_ARG_ENABLE([dmem],
 m4_define([AC_ARG_ADAPTERS], [
   m4_foreach([adapter], [$1],
 	[AC_ARG_ENABLE(ADAPTER_OPT([adapter]),
-		AS_HELP_STRING([--enable-ADAPTER_OPT([adapter])],
+		AS_HELP_STRING([--enable-ADAPTER_OPT([adapter])[[[=yes/no/auto]]]],
 			[Enable building support for the ]ADAPTER_DESC([adapter])[ (default is $2)]),
-		[], [ADAPTER_VAR([adapter])=$2])
+		[case "${enableval}" in
+			yes|no|auto) ;;
+			*) AC_MSG_ERROR([Option --enable-ADAPTER_OPT([adapter]) has invalid value "${enableval}".]) ;;
+			esac],
+		[ADAPTER_VAR([adapter])=$2])
   ])
 ])
 
@@ -268,7 +288,7 @@ AC_ARG_ADAPTERS([
   HIDAPI_ADAPTERS,
   HIDAPI_USB1_ADAPTERS,
   LIBFTDI_ADAPTERS,
-  LIBFTDI_USB1_ADAPTERS
+  LIBFTDI_USB1_ADAPTERS,
   LIBGPIOD_ADAPTERS,
   SERIAL_PORT_ADAPTERS,
   PCIE_ADAPTERS,
@@ -357,10 +377,6 @@ AS_CASE([$host_os],
       AC_MSG_ERROR([sysfsgpio is only available on linux])
     ])
 
-    AS_IF([test "x$enable_linuxgpiod" = "xyes"], [
-      AC_MSG_ERROR([linuxgpiod is only available on linux])
-    ])
-
     AS_CASE([$host_os], [freebsd*], [],
     [
       AS_IF([test "x$build_rshim" = "xyes"], [
@@ -399,6 +415,8 @@ AS_CASE(["${host_cpu}"],
     parport_use_ppdev=yes
 ])
 
+can_build_buspirate=yes
+
 AS_CASE([$host],
   [*-cygwin*], [
     is_win32=yes
@@ -430,12 +448,12 @@ AS_CASE([$host],
     ])
     parport_use_giveio=yes
 
-    AS_IF([test "x$enable_buspirate" = "xyes"], [
-      AC_MSG_ERROR([buspirate currently not supported by MinGW32 hosts])
+    AS_IF([test "x$ADAPTER_VAR([buspirate])" = "xyes"], [
+      AC_MSG_ERROR([The Bus Pirate adapter is currently not supported by MinGW32 hosts.])
     ])
 
     # In case enable_buspirate=auto, make sure it will not be built.
-    enable_buspirate=no
+    can_build_buspirate=no
 
     AC_SUBST([HOST_CPPFLAGS], ["-D__USE_MINGW_ANSI_STDIO -DFD_SETSIZE=128"])
   ],
@@ -579,12 +597,6 @@ AS_IF([test "x$build_gw16012" = "xyes"], [
   AC_DEFINE([BUILD_GW16012], [0], [0 if you don't want the Gateworks GW16012 driver.])
 ])
 
-AS_IF([test "x$enable_buspirate" != "xno"], [
-  AC_DEFINE([BUILD_BUSPIRATE], [1], [1 if you want the Buspirate JTAG driver.])
-], [
-  AC_DEFINE([BUILD_BUSPIRATE], [0], [0 if you don't want the Buspirate JTAG driver.])
-])
-
 AS_IF([test "x$use_internal_jimtcl" = "xyes"], [
   AS_IF([test -f "$srcdir/jimtcl/configure"], [
     AS_IF([test "x$use_internal_jimtcl_maintainer" = "xyes"], [
@@ -697,7 +709,7 @@ m4_define([PROCESS_ADAPTERS], [
 		])
 	], [
 		AS_IF([test "x$ADAPTER_VAR([adapter])" = "xyes"], [
-			AC_MSG_ERROR([$3 is required for [adapter] ADAPTER_DESC([adapter]).])
+			AC_MSG_ERROR([$3 is required for [adapter] "ADAPTER_DESC([adapter])".])
 		])
 		ADAPTER_VAR([adapter])=no
 		AC_DEFINE([BUILD_]ADAPTER_SYM([adapter]), [0], [0 if you do not want the ]ADAPTER_DESC([adapter]).)
@@ -711,9 +723,11 @@ PROCESS_ADAPTERS([HIDAPI_ADAPTERS], ["x$use_hidapi" = "xyes"], [hidapi])
 PROCESS_ADAPTERS([HIDAPI_USB1_ADAPTERS], ["x$use_hidapi" = "xyes" -a "x$use_libusb1" = "xyes"], [hidapi and libusb-1.x])
 PROCESS_ADAPTERS([LIBFTDI_ADAPTERS], ["x$use_libftdi" = "xyes"], [libftdi])
 PROCESS_ADAPTERS([LIBFTDI_USB1_ADAPTERS], ["x$use_libftdi" = "xyes" -a "x$use_libusb1" = "xyes"], [libftdi and libusb-1.x])
-PROCESS_ADAPTERS([LIBGPIOD_ADAPTERS], ["x$use_libgpiod" = "xyes"], [libgpiod])
+PROCESS_ADAPTERS([LIBGPIOD_ADAPTERS], ["x$use_libgpiod" = "xyes"], [Linux libgpiod])
 PROCESS_ADAPTERS([LIBJAYLINK_ADAPTERS], ["x$use_internal_libjaylink" = "xyes" -o "x$use_libjaylink" = "xyes"], [libjaylink-0.2])
 PROCESS_ADAPTERS([PCIE_ADAPTERS], ["x$is_linux" = "xyes"], [Linux build])
+PROCESS_ADAPTERS([SERIAL_PORT_ADAPTERS], ["x$can_build_buspirate" = "xyes"],
+                                         [internal error: validation should happen beforehand])
 PROCESS_ADAPTERS([DUMMY_ADAPTER], [true], [unused])
 
 AS_IF([test "x$enable_linuxgpiod" != "xno"], [
@@ -768,7 +782,6 @@ AM_CONDITIONAL([USB_BLASTER_DRIVER], [test "x$enable_usb_blaster" != "xno" -o "x
 AM_CONDITIONAL([AMTJTAGACCEL], [test "x$build_amtjtagaccel" = "xyes"])
 AM_CONDITIONAL([GW16012], [test "x$build_gw16012" = "xyes"])
 AM_CONDITIONAL([REMOTE_BITBANG], [test "x$build_remote_bitbang" = "xyes"])
-AM_CONDITIONAL([BUSPIRATE], [test "x$enable_buspirate" != "xno"])
 AM_CONDITIONAL([SYSFSGPIO], [test "x$build_sysfsgpio" = "xyes"])
 AM_CONDITIONAL([USE_LIBUSB1], [test "x$use_libusb1" = "xyes"])
 AM_CONDITIONAL([IS_CYGWIN], [test "x$is_cygwin" = "xyes"])
@@ -788,6 +801,8 @@ AM_CONDITIONAL([INTERNAL_JIMTCL], [test "x$use_internal_jimtcl" = "xyes"])
 AM_CONDITIONAL([HAVE_JIMTCL_PKG_CONFIG], [test "x$have_jimtcl_pkg_config" = "xyes"])
 AM_CONDITIONAL([INTERNAL_LIBJAYLINK], [test "x$use_internal_libjaylink" = "xyes"])
 
+AM_CONDITIONAL([USE_GCOV], [test "x$enable_gcov" = "xyes"])
+
 # Look for environ alternatives.  Possibility #1: is environ in unistd.h or stdlib.h?
 AC_MSG_CHECKING([for environ in unistd.h and stdlib.h])
 AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
@@ -863,7 +878,8 @@ m4_foreach([adapter], [USB1_ADAPTERS,
 	LIBGPIOD_ADAPTERS,
 	LIBJAYLINK_ADAPTERS, PCIE_ADAPTERS, SERIAL_PORT_ADAPTERS,
 	DUMMY_ADAPTER,
-	OPTIONAL_LIBRARIES],
+	OPTIONAL_LIBRARIES,
+	COVERAGE],
 	[s=m4_format(["%-40s"], ADAPTER_DESC([adapter]))
 	AS_CASE([$ADAPTER_VAR([adapter])],
 		[auto], [
@@ -874,6 +890,11 @@ m4_foreach([adapter], [USB1_ADAPTERS,
 		],
 		[no], [
 			echo "$s"no
+		],
+		[
+			AC_MSG_ERROR(m4_normalize([
+				Error in [adapter] "ADAPTER_ARG([adapter])": Variable "ADAPTER_VAR([adapter])"
+				has invalid value "$ADAPTER_VAR([adapter])".]))
 	])
 ])
 echo

src/helper/jim-nvp.h

@@ -19,6 +19,7 @@
 #ifndef OPENOCD_HELPER_JIM_NVP_H
 #define OPENOCD_HELPER_JIM_NVP_H
 
+#include <stdbool.h>
 #include <jim.h>
 
 /** Name Value Pairs, aka: NVP
@@ -136,7 +137,7 @@ struct jim_getopt_info {
 	Jim_Interp *interp;
 	int argc;
 	Jim_Obj *const *argv;
-	int isconfigure;		/* non-zero if configure */
+	bool is_configure;
 };
 
 /** GetOpt - how to.

src/jtag/adapter.c

@@ -66,6 +66,8 @@ static const struct gpio_map {
 	[ADAPTER_GPIO_IDX_LED] = { "led", ADAPTER_GPIO_DIRECTION_OUTPUT, true, true, },
 };
 
+static int adapter_config_khz(unsigned int khz);
+
 bool is_adapter_initialized(void)
 {
 	return adapter_config.adapter_initialized;
@@ -245,7 +247,8 @@ static int adapter_set_speed(int speed)
 	return is_adapter_initialized() ? adapter_driver->speed(speed) : ERROR_OK;
 }
 
-int adapter_config_khz(unsigned int khz)
+/** Attempt to configure the adapter for the specified kHz. */
+static int adapter_config_khz(unsigned int khz)
 {
 	LOG_DEBUG("handle adapter khz");
 	adapter_config.clock_mode = CLOCK_MODE_KHZ;

src/jtag/adapter.h

@@ -97,9 +97,6 @@ int adapter_get_speed(int *speed);
  */
 int adapter_get_speed_readable(int *speed);
 
-/** Attempt to configure the adapter for the specified kHz. */
-int adapter_config_khz(unsigned int khz);
-
 /**
  * Attempt to enable RTCK/RCLK. If that fails, fallback to the
  * specified frequency.

src/jtag/core.c

@@ -51,6 +51,8 @@ static void jtag_add_scan_check(struct jtag_tap *active,
 		tap_state_t state),
 		int in_num_fields, struct scan_field *in_fields, tap_state_t state);
 
+static int jtag_error_clear(void);
+
 /**
  * The jtag_error variable is set when an error occurs while executing
  * the queue.  Application code may set this using jtag_set_error(),
@@ -127,7 +129,11 @@ void jtag_set_error(int error)
 	jtag_error = error;
 }
 
-int jtag_error_clear(void)
+/**
+ * Resets jtag_error to ERROR_OK, returning its previous value.
+ * @returns The previous value of @c jtag_error.
+ */
+static int jtag_error_clear(void)
 {
 	int temp = jtag_error;
 	jtag_error = ERROR_OK;
@@ -186,7 +192,7 @@ struct jtag_tap *jtag_all_taps(void)
 	return __jtag_all_taps;
 };
 
-unsigned int jtag_tap_count(void)
+static unsigned int jtag_tap_count(void)
 {
 	struct jtag_tap *t = jtag_all_taps();
 	unsigned int n = 0;