[PATCH v2 0/3] Add displaying a version number in imx-usb-loader

[PATCH v2 0/3] Add displaying a version number in imx-usb-loader

[Johannes Zink]

This series adds displaying a version number in imx-usb-loader for
easier debugging.

The first two patches add minor cleanup, while the third actually adds
the code for displaying the version number.

The result will look like
$./imx-usb-loader --version
./imx-usb-loader 2023.04.0-00210-g057705fb211a-dirty

Happy hacking
Johannes

Changelog:
  v1 -> v2: worked in Ahmad's review findings. Thanks for reviewing my
            patches!
            - do not rely on autogenerated headers to keep tool buildable
	      outside of KBuild environment
	    - remove double exit when displaying version number
	    - fixed errornous commit subject
	    - added comment in commit message why removing the exit()
	      call in a function is ok, even when not explicitely adding
	      it to all call sites


Johannes Zink (3):
  imx-usb-loader: exit with success when displaying help on purpose
  imx-usb-loader: use proper return code macros
  imx-usb-loader: add commandline option for displaying version number

 scripts/imx/Makefile         |  2 +-
 scripts/imx/imx-usb-loader.c | 30 +++++++++++++++++++++++++-----
 2 files changed, 26 insertions(+), 6 deletions(-)

-- 
2.39.2

[PATCH v2 1/3] imx-usb-loader: exit with success when displaying help on purpose

[Johannes Zink]

Previously, whenever the usage was displayed, the imx-usb-loader exited
with error code.

When the usage is displayed due to invalid tool invocation, returning an
error code is valid behaviour, but when displaying the usage with the -h
command line option, success should be returned.

The other usage() callsite in the code already has an exit(1) following
it, which is why the exit call can be removed from the usage function
without introducing any behavioural change there.

Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
Changelog:

 v1 -> v2: - fixed erronous commit subject
           - added comment in commit message why removing the exit()
	     call is from the usage() function is valid, even when not
	     explicitely adding an exit() call at all call sites

 scripts/imx/imx-usb-loader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index 41d57906c752..17b83b611765 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -1527,7 +1527,6 @@ static void usage(const char *prgname)
 		"-s           skip DCD included in image\n"
 		"-v           verbose (give multiple times to increase)\n"
 		"-h           this help\n", prgname);
-	exit(1);
 }
 
 int main(int argc, char *argv[])
@@ -1558,6 +1557,7 @@ int main(int argc, char *argv[])
 			break;
 		case 'h':
 			usage(argv[0]);
+			exit(0);
 		case 'd':
 			devtype = optarg;
 			break;
-- 
2.39.2

[PATCH v2 2/3] imx-usb-loader: use proper return code macros

[Johannes Zink]

It is considered good practice to use EXIT_SUCCESS and EXIT_FAILURE
instead of hardcoding magic values.

No functional changes.

Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
 scripts/imx/imx-usb-loader.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index 17b83b611765..e49c0bea6ca4 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -1557,7 +1557,7 @@ int main(int argc, char *argv[])
 			break;
 		case 'h':
 			usage(argv[0]);
-			exit(0);
+			exit(EXIT_SUCCESS);
 		case 'd':
 			devtype = optarg;
 			break;
@@ -1571,13 +1571,13 @@ int main(int argc, char *argv[])
 			w.do_dcd_once = 0;
 			break;
 		default:
-			exit(1);
+			exit(EXIT_FAILURE);
 		}
 	}
 
 	if (devtype && strcmp(devtype, "list") == 0) {
 		list_imx_device_types();
-		exit(0);
+		exit(EXIT_SUCCESS);
 	}
 
 	if (devtype && !devpath) {
@@ -1587,7 +1587,7 @@ int main(int argc, char *argv[])
 	if (optind == argc) {
 		fprintf(stderr, "no filename given\n");
 		usage(argv[0]);
-		exit(1);
+		exit(EXIT_FAILURE);
 	}
 
 	w.plug = 1;
-- 
2.39.2

[PATCH v2 3/3] imx-usb-loader: add commandline option for displaying version number

[Johannes Zink]

For debugging purposes of the imx-usb-loader it can be helpful to
display the version number as a commandline option.

If built from outside the KBuild infrastructure, the version string
cannot be fetched from the generated utsrelease.h header the version
number needs to be defined by setting the UTS_VERSION macro. If the
macro is not set on an out-of-KBuild build, the version number "unknown"
is displayed.

Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
Changelog:

v1 -> v2: - fixed tool relying on KBuild-infrastructure-generated header
            file to allow build outside of a KBuild environment
	  - remove unneeded double exit() call when displaying the
	    version number

 scripts/imx/Makefile         |  2 +-
 scripts/imx/imx-usb-loader.c | 22 +++++++++++++++++++++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/scripts/imx/Makefile b/scripts/imx/Makefile
index dbfa82910a55..b3be3886d8eb 100644
--- a/scripts/imx/Makefile
+++ b/scripts/imx/Makefile
@@ -3,7 +3,7 @@
 hostprogs-always-$(CONFIG_ARCH_IMX_IMXIMAGE)	+= imx-image
 hostprogs-always-$(CONFIG_ARCH_IMX_USBLOADER)	+= imx-usb-loader
 
-HOSTCFLAGS_imx-usb-loader.o = `pkg-config --cflags libusb-1.0`
+HOSTCFLAGS_imx-usb-loader.o = `pkg-config --cflags libusb-1.0` -include $(objtree)/include/generated/utsrelease.h
 HOSTLDLIBS_imx-usb-loader  = `pkg-config --libs libusb-1.0`
 
 imx-usb-loader-target-userccflags += `$(CROSS_PKG_CONFIG) --cflags libusb-1.0`
diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index e49c0bea6ca4..5f9c7ff3a458 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -46,6 +46,10 @@
 #define FT_DCD	0xee
 #define FT_LOAD_ONLY	0x00
 
+#ifndef UTS_RELEASE
+#define UTS_RELEASE "unknown"
+#endif
+
 /*
  * comment from libusb:
  * As per the USB 3.0 specs, the current maximum limit for the depth is 7.
@@ -1526,9 +1530,15 @@ static void usage(const char *prgname)
 		"-p <devpath> Specify device path: <bus>-<port>[.<port>]...\n"
 		"-s           skip DCD included in image\n"
 		"-v           verbose (give multiple times to increase)\n"
+		"--version    display version number\n"
 		"-h           this help\n", prgname);
 }
 
+static void version(const char *prgname)
+{
+	fprintf(stderr, "%s %s\n", prgname, UTS_RELEASE);
+}
+
 int main(int argc, char *argv[])
 {
 	libusb_device **devs;
@@ -1544,10 +1554,20 @@ int main(int argc, char *argv[])
 	char *initfile = NULL;
 	char *devpath = NULL;
 	char *devtype = NULL;
+	int opt_version = 0;
+	struct option long_options[] = {
+		{"version", no_argument, &opt_version, 1},
+		{ }
+	};
 
 	w.do_dcd_once = 1;
 
-	while ((opt = getopt(argc, argv, "cvhd:i:p:s")) != -1) {
+	while ((opt = getopt_long(argc, argv, "cvhd:i:p:s", long_options, NULL)) != -1) {
+		if (opt_version) {
+			version(argv[0]);
+			exit(EXIT_SUCCESS);
+		}
+
 		switch (opt) {
 		case 'c':
 			verify = 1;
-- 
2.39.2

Re: [PATCH v2 0/3] Add displaying a version number in imx-usb-loader

[Sascha Hauer]

On Fri, May 05, 2023 at 05:52:45PM +0200, Johannes Zink wrote:
> This series adds displaying a version number in imx-usb-loader for
> easier debugging.
> 
> The first two patches add minor cleanup, while the third actually adds
> the code for displaying the version number.
> 
> The result will look like
> $./imx-usb-loader --version
> ./imx-usb-loader 2023.04.0-00210-g057705fb211a-dirty
> 
> Happy hacking
> Johannes
> 
> Changelog:
>   v1 -> v2: worked in Ahmad's review findings. Thanks for reviewing my
>             patches!
>             - do not rely on autogenerated headers to keep tool buildable
> 	      outside of KBuild environment
> 	    - remove double exit when displaying version number
> 	    - fixed errornous commit subject
> 	    - added comment in commit message why removing the exit()
> 	      call in a function is ok, even when not explicitely adding
> 	      it to all call sites
> 
> 
> Johannes Zink (3):
>   imx-usb-loader: exit with success when displaying help on purpose
>   imx-usb-loader: use proper return code macros
>   imx-usb-loader: add commandline option for displaying version number

Applied, thanks

Sascha

> 
>  scripts/imx/Makefile         |  2 +-
>  scripts/imx/imx-usb-loader.c | 30 +++++++++++++++++++++++++-----
>  2 files changed, 26 insertions(+), 6 deletions(-)
> 
> -- 
> 2.39.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |