Skip to contentSkip to conversation

mkimage: add "-V" option to print version information

PATCH Superseded Wolfgang Denk 3 participants 18 messages Feb 11, 2011 – Apr 12, 2011 Kbuild

lore
Share & tools
lore

Wolfgang Denk

author
#0 lore raw

PATCH 1/1 mkimage: add "-V" option to print version information

Signed-off-by Wolfgang Denk <[email protected]>
---
 tools/mkimage.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
tools/mkimage.c +6 -0
index f5859d7..788484d 100644
@@ -23,6 +23,7 @@
 
 #include "mkimage.h"
 #include <image.h>
+#include <version.h>
 
 static void copy_file(int, const char *, int);
 static void usage(void);

@@ -246,6 +247,10 @@ main (int argc, char **argv)
 			case 'v':
 				params.vflag++;
 				break;
+			case 'V':
+				printf("mkimage version %s\n",
+					U_BOOT_VERSION + 7);
+				exit(EXIT_SUCCESS);
 			case 'x':
 				params.xflag++;
 				break;

@@ -590,6 +595,7 @@ usage ()
 		params.cmdname);
 	fprintf (stderr, "       %s [-D dtc_options] -f fit-image.its fit-image\n",
 		params.cmdname);
+	fprintf (stderr, "       %s -V ==> print version information and exit\n");
 
 	exit (EXIT_FAILURE);
 }
Signature · 1 line
1.7.4
author
#1 lore raw

PATCH v2 1/1 mkimage: add "-V" option to print version information

Signed-off-by Wolfgang Denk <[email protected]>

---
v2: fix missing argument to printf() call.

 tools/mkimage.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
tools/mkimage.c +7 -0
index f5859d7..127be57 100644
@@ -23,6 +23,7 @@
 
 #include "mkimage.h"
 #include <image.h>
+#include <version.h>
 
 static void copy_file(int, const char *, int);
 static void usage(void);

@@ -246,6 +247,10 @@ main (int argc, char **argv)
 			case 'v':
 				params.vflag++;
 				break;
+			case 'V':
+				printf("mkimage version %s\n",
+					U_BOOT_VERSION + 7);
+				exit(EXIT_SUCCESS);
 			case 'x':
 				params.xflag++;
 				break;

@@ -590,6 +595,8 @@ usage ()
 		params.cmdname);
 	fprintf (stderr, "       %s [-D dtc_options] -f fit-image.its fit-image\n",
 		params.cmdname);
+	fprintf (stderr, "       %s -V ==> print version information and exit\n",
+		params.cmdname);
 
 	exit (EXIT_FAILURE);
 }
Signature · 1 line
1.7.4
#2 lore raw

PATCH v2 1/1 mkimage: add "-V" option to print version information

Wolfgang,

Please ignore my previous post on V1, I had not seen V2. My comment holds, though:

Le 11/02/2011 13:22, Wolfgang Denk a ?crit :

Quoted 20 lines
Signed-off-by Wolfgang Denk<[email protected]>

---
v2: fix missing argument to printf() call.

  tools/mkimage.c |    7 +++++++
  1 files changed, 7 insertions(+), 0 deletions(-)
tools/mkimage.c +1 -0
index f5859d7..127be57 100644
@@ -23,6 +23,7 @@
 
  #include "mkimage.h"
  #include<image.h>
+#include<version.h>
 
  static void copy_file(int, const char *, int);
  static void usage(void);

tools/mkimage.c main (int argc, char **argv) · line 252

Quoted 6 lines
tools/mkimage.c +3 -0
@@ -246,6 +247,10 @@ main (int argc, char **argv)
  			case 'v':
  				params.vflag++;
  				break;
+			case 'V':
+				printf("mkimage version %s\n",
+					U_BOOT_VERSION + 7);

If that magic number 7 (and the addition, as well) has any reason, it should at least be explained in a short comment.

tools/mkimage.c usage () · line 602

Quoted 13 lines
tools/mkimage.c +3 -0
+				exit(EXIT_SUCCESS);
  			case 'x':
  				params.xflag++;
  				break;

@@ -590,6 +595,8 @@ usage ()
  		params.cmdname);
  	fprintf (stderr, "       %s [-D dtc_options] -f fit-image.its fit-image\n",
  		params.cmdname);
+	fprintf (stderr, "       %s -V ==>  print version information and exit\n",
+		params.cmdname);
 
  	exit (EXIT_FAILURE);
  }

Amicalement,

Signature · 1 line
Albert.
author
#3 lore raw

PATCH v2 1/1 mkimage: add "-V" option to print version information

Dear Albert ARIBAUD,

Quoted 3 lines · In message <[email protected]> you wrote:

Please ignore my previous post on V1, I had not seen V2. My comment holds, though:

...

Quoted 6 lines

tools/mkimage.c main (int argc, char **argv) · line 252

Quoted 3 lines
tools/mkimage.c +3 -0
+			case 'V':
+				printf("mkimage version %s\n",
+					U_BOOT_VERSION + 7);

If that magic number 7 (and the addition, as well) has any reason, it > should at least be explained in a short comment.

Heh... I though I could leave this as an exercide for the reader ;-)

New version following...

Thanks for pointing out.

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"The hottest places in Hell are reserved for those who, in  times  of
moral crisis, preserved their neutrality."                    - Dante
#4 lore raw

PATCH 1/1 mkimage: add "-V" option to print version information

In reply to Wolfgang Denk

Hi Wolfgang,

Le 11/02/2011 09:56, Wolfgang Denk a ?crit :

Quoted 18 lines
Signed-off-by Wolfgang Denk<[email protected]>
---
  tools/mkimage.c |    6 ++++++
  1 files changed, 6 insertions(+), 0 deletions(-)
tools/mkimage.c +1 -0
index f5859d7..788484d 100644
@@ -23,6 +23,7 @@
 
  #include "mkimage.h"
  #include<image.h>
+#include<version.h>
 
  static void copy_file(int, const char *, int);
  static void usage(void);

tools/mkimage.c main (int argc, char **argv) · line 252

Quoted 6 lines
tools/mkimage.c +3 -0
@@ -246,6 +247,10 @@ main (int argc, char **argv)
  			case 'v':
  				params.vflag++;
  				break;
+			case 'V':
+				printf("mkimage version %s\n",
+					U_BOOT_VERSION + 7);

That 7 is a pretty awful magic number here, isn't it? At least if there's a sane reason for this it should be summarized in a short comment.

tools/mkimage.c usage () · line 601

Quoted 12 lines
tools/mkimage.c +2 -0
+				exit(EXIT_SUCCESS);
  			case 'x':
  				params.xflag++;
  				break;

@@ -590,6 +595,7 @@ usage ()
  		params.cmdname);
  	fprintf (stderr, "       %s [-D dtc_options] -f fit-image.its fit-image\n",
  		params.cmdname);
+	fprintf (stderr, "       %s -V ==>  print version information and exit\n");
 
  	exit (EXIT_FAILURE);
  }

Amicalement,

Signature · 1 line
Albert.
author
#5 lore raw

PATCH v3 1/1 mkimage: add "-V" option to print version information

In reply to Wolfgang Denk
Signed-off-by Wolfgang Denk <[email protected]>

---
v2: fix missing argument to printf() call.
v3: explain the magic "+ 7" offset into the version string

 tools/mkimage.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)
tools/mkimage.c +11 -0
index f5859d7..febb536 100644
@@ -23,6 +23,7 @@
 
 #include "mkimage.h"
 #include <image.h>
+#include <version.h>
 
 static void copy_file(int, const char *, int);
 static void usage(void);

@@ -246,6 +247,14 @@ main (int argc, char **argv)
 			case 'v':
 				params.vflag++;
 				break;
+			case 'V':
+				/*
+				 * Skip the "U-Boot " part in
+				 * U_BOOT_VERSION by adding 7
+				 */
+				printf("mkimage version %s\n",
+					U_BOOT_VERSION + 7);
+				exit(EXIT_SUCCESS);
 			case 'x':
 				params.xflag++;
 				break;

@@ -590,6 +599,8 @@ usage ()
 		params.cmdname);
 	fprintf (stderr, "       %s [-D dtc_options] -f fit-image.its fit-image\n",
 		params.cmdname);
+	fprintf (stderr, "       %s -V ==> print version information and exit\n",
+		params.cmdname);
 
 	exit (EXIT_FAILURE);
 }
Signature · 1 line
1.7.4
#6 lore raw

PATCH v3 1/1 mkimage: add "-V" option to print version information

tools/mkimage.c main (int argc, char **argv) · line 256

Quoted 7 lines · On Fri, 11 Feb 2011 23:41:43 +0100 Wolfgang Denk <[email protected]> wrote:
tools/mkimage.c +7 -0
+			case 'V':
+				/*
+				 * Skip the "U-Boot " part in
+				 * U_BOOT_VERSION by adding 7
+				 */
+				printf("mkimage version %s\n",
+					U_BOOT_VERSION + 7);

I'd have done it without magic nor comments as

U_BOOT_VERSION[sizeof("U-Boot ") + 1]

or even

U_BOOT_VERSION[strlen("U-Boot ")]

Kim

#7 lore raw

PATCH v3 1/1 mkimage: add "-V" option to print version information

Le 12/02/2011 00:11, Kim Phillips a ?crit :

Quoted 10 lines

tools/mkimage.c main (int argc, char **argv) · line 256

Quoted 7 lines · On Fri, 11 Feb 2011 23:41:43 +0100 Wolfgang Denk<[email protected]> wrote:
tools/mkimage.c +7 -0
+			case 'V':
+				/*
+				 * Skip the "U-Boot " part in
+				 * U_BOOT_VERSION by adding 7
+				 */
+				printf("mkimage version %s\n",
+					U_BOOT_VERSION + 7);

Now I get it. :)

I might argue that this is kind of a hack, and that rather than trying to prune the U_BOOT_VERSION string, one should define two macros, for instance:

	#define PLAIN_VERSION "whatever"
	#define U_BOOT_VERSION "U-Boot " PLAIN_VERSION

... which has the advantage of not affecting the existing code, or a cleaner, but more invasive, change...

	#define U_BOOT_VERSION "whatever" /* without "U-Boot " */
	#define U_BOOT_VERSION_BANNER "U-Boot " U_BOOT_VERSION

... and use the right macro for the right need.

Quoted 7 lines

I'd have done it without magic nor comments as

U_BOOT_VERSION[sizeof("U-Boot ") + 1]

or even

U_BOOT_VERSION[strlen("U-Boot ")]

The second one calls strlen() at run-time, plus it allocates the "U-Boot " string for no justifiable reason -- assuming the first one can be compile-time evaluated by the compiler, of course.

Quoted 1 line

Kim

Amicalement,

Signature · 1 line
Albert.
author
#8 lore raw

PATCH v3 1/1 mkimage: add "-V" option to print version information

Dear Albert ARIBAUD,

Quoted 7 lines · In message <[email protected]> you wrote:

I might argue that this is kind of a hack, and that rather than trying to prune the U_BOOT_VERSION string, one should define two macros, for instance:

	#define PLAIN_VERSION "whatever"
	#define U_BOOT_VERSION "U-Boot " PLAIN_VERSION

I know this works find in C, but I'm not absolutely sure if this is acceptable with all assemblers.

But I get the idea...

Updated patch follows.

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
People seldom know what they want until you give them what  they  ask
for.
#9 lore raw

PATCH v3 1/1 mkimage: add "-V" option to print version information

In reply to Albert ARIBAUD
Quoted 5 lines · On Sat, 12 Feb 2011 07:47:38 +0100 Albert ARIBAUD <[email protected]>…

Le 12/02/2011 00:11, Kim Phillips a ?crit :

Quoted 4 lines

I'd have done it without magic nor comments as

U_BOOT_VERSION[sizeof("U-Boot ") + 1]

Quoted 7 lines
Quoted 3 lines

or even

U_BOOT_VERSION[strlen("U-Boot ")]

The second one calls strlen() at run-time, plus it allocates the "U-Boot " string for no justifiable reason -- assuming the first one can be compile-time evaluated by the compiler, of course.

no, the compiler evaluates the strlen at compile time and eliminates the need to allocate the "U-Boot " string in both cases.

that's not to say that there aren't a couple of gaffes above: the sizeof needs a - 1 instead of a + 1 (because it includes the trailing '\0'), and both expressions need to be prepended with an '&'. So, we have something like:

&U_BOOT_VERSION[sizeof("U-Boot ") - 1]

or

&U_BOOT_VERSION[strlen("U-Boot ")]

Kim

author
#10 lore raw

PATCH v3 1/1 mkimage: add "-V" option to print version information

In reply to Kim Phillips

Dear Kim Phillips,

Quoted 13 lines · In message <[email protected]> you wrote:

tools/mkimage.c main (int argc, char **argv) · line 256

Quoted 7 lines · On Fri, 11 Feb 2011 23:41:43 +0100 Wolfgang Denk <[email protected]> wrote:
tools/mkimage.c +7 -0
+			case 'V':
+				/*
+				 * Skip the "U-Boot " part in
+				 * U_BOOT_VERSION by adding 7
+				 */
+				printf("mkimage version %s\n",
+					U_BOOT_VERSION + 7);

I'd have done it without magic nor comments as

Quoted 5 lines

U_BOOT_VERSION[sizeof("U-Boot ") + 1]

or even

U_BOOT_VERSION[strlen("U-Boot ")]

Hm... I would have cleaned up such locations in U-Boot as part of my v4 patch that introduces PLAIN_VERSION - if I had found any such code. Am I missing something?

Best regards,

Wolfgang Denk

Signature · 4 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"I haven't lost my mind - it's backed up on tape somewhere."
#11 lore raw

PATCH v3 1/1 mkimage: add "-V" option to print version information

Quoted 18 lines · On Sat, 12 Feb 2011 10:37:16 +0100 Wolfgang Denk <[email protected]> wrote:

Dear Kim Phillips,

Quoted 15 lines · In message <[email protected]> you wrote:

tools/mkimage.c main (int argc, char **argv) · line 256

Quoted 7 lines · On Fri, 11 Feb 2011 23:41:43 +0100 Wolfgang Denk <[email protected]> wrote:
tools/mkimage.c +7 -0
+			case 'V':
+				/*
+				 * Skip the "U-Boot " part in
+				 * U_BOOT_VERSION by adding 7
+				 */
+				printf("mkimage version %s\n",
+					U_BOOT_VERSION + 7);

I'd have done it without magic nor comments as

U_BOOT_VERSION[sizeof("U-Boot ") + 1]

Quoted 7 lines
Quoted 3 lines

or even

U_BOOT_VERSION[strlen("U-Boot ")]

Hm... I would have cleaned up such locations in U-Boot as part of my v4 patch that introduces PLAIN_VERSION - if I had found any such code. Am I missing something?

ok, apart from the missing & and +/- gaffes, what are you talking about? The code does exactly what the + 7 does, except it's more self-explanatory.

Kim

author
#12 lore raw

PATCH v3 1/1 mkimage: add "-V" option to print version information

Dear Kim Phillips,

Quoted 2 lines · In message <[email protected]> you wrote:
Quoted 2 lines
Quoted 2 lines

I'd have done it without magic nor comments as

...

Quoted 7 lines
Quoted 3 lines

Hm... I would have cleaned up such locations in U-Boot as part of my v4 patch that introduces PLAIN_VERSION - if I had found any such code. Am I missing something?

ok, apart from the missing & and +/- gaffes, what are you talking about? The code does exactly what the + 7 does, except it's more self-explanatory.

Sorry, my fault. I've misread your "I'd have done it" as "I have done it" but did not find any such code. Please ignore me.

Best regards,

Wolfgang Denk

Signature · 8 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Ill-chosen abstraction is particularly evident in the design  of  the
ADA  runtime  system.  The  interface to the ADA runtime system is so
opaque that it is impossible to model  or  predict  its  performance,
making it effectively useless for real-time systems.
                              - Marc D.  Donner and David H. Jameson.
author
#13 lore raw

PATCH v4 1/1 mkimage: add "-V" option to print version information

In reply to Wolfgang Denk
Signed-off-by Wolfgang Denk <[email protected]>

---
v2: fix missing argument to printf() call.
v3: explain the magic "+ 7" offset into the version string
v3: avoid offset into U_BOOT_VERSION string completely and define
a new PLAIN_VERSION variable instead; this has the benefit that it can be used in other places as well where such version information might be needed (fw_{set,print}env etc. comes to mind).

 Makefile        |    8 ++++++--
 tools/mkimage.c |    6 ++++++
 2 files changed, 12 insertions(+), 2 deletions(-)
Makefile +6 -2
index 05b404d..8721b59 100644
@@ -414,8 +414,12 @@ $(U_BOOT_ONENAND):	$(ONENAND_IPL) $(obj)u-boot.bin
 		cat $(ONENAND_BIN) $(obj)u-boot.bin > $(obj)u-boot-onenand.bin
 
 $(VERSION_FILE):
-		@( printf '#define U_BOOT_VERSION "U-Boot %s%s"\n' "$(U_BOOT_VERSION)" \
-		 '$(shell $(TOPDIR)/tools/setlocalversion $(TOPDIR))' ) > [email protected]
+		@( localvers='$(shell $(TOPDIR)/tools/setlocalversion $(TOPDIR))' ; \
+		   printf '#define PLAIN_VERSION "%s%s"\n' \
+		   	"$(U_BOOT_VERSION)" "$${localvers}" ; \
+		   printf '#define U_BOOT_VERSION "U-Boot %s%s"\n' \
+		   	"$(U_BOOT_VERSION)" "$${localvers}" ; \
+		) > [email protected]
 		@( printf '#define CC_VERSION_STRING "%s"\n' \
 		 '$(shell $(CC) --version | head -n 1)' )>>  [email protected]
 		@( printf '#define LD_VERSION_STRING "%s"\n' \
tools/mkimage.c +6 -0
index f5859d7..60f7263 100644
@@ -23,6 +23,7 @@
 
 #include "mkimage.h"
 #include <image.h>
+#include <version.h>
 
 static void copy_file(int, const char *, int);
 static void usage(void);

@@ -246,6 +247,9 @@ main (int argc, char **argv)
 			case 'v':
 				params.vflag++;
 				break;
+			case 'V':
+				printf("mkimage version %s\n", PLAIN_VERSION);
+				exit(EXIT_SUCCESS);
 			case 'x':
 				params.xflag++;
 				break;

@@ -590,6 +594,8 @@ usage ()
 		params.cmdname);
 	fprintf (stderr, "       %s [-D dtc_options] -f fit-image.its fit-image\n",
 		params.cmdname);
+	fprintf (stderr, "       %s -V ==> print version information and exit\n",
+		params.cmdname);
 
 	exit (EXIT_FAILURE);
 }
Signature · 1 line
1.7.4
#14 lore raw

PATCH v4 1/1 mkimage: add "-V" option to print version information

Makefile $(U_BOOT_ONENAND): $(ONENAND_IPL) $(obj)u-boot.bin · line 422

Quoted 8 lines · On Sat, 12 Feb 2011 10:37:11 +0100 Wolfgang Denk <[email protected]> wrote:
Makefile +6 -2
-		@( printf '#define U_BOOT_VERSION "U-Boot %s%s"\n' "$(U_BOOT_VERSION)" \
-		 '$(shell $(TOPDIR)/tools/setlocalversion $(TOPDIR))' ) > [email protected]
+		@( localvers='$(shell $(TOPDIR)/tools/setlocalversion $(TOPDIR))' ; \
+		   printf '#define PLAIN_VERSION "%s%s"\n' \
+		   	"$(U_BOOT_VERSION)" "$${localvers}" ; \
+		   printf '#define U_BOOT_VERSION "U-Boot %s%s"\n' \
+		   	"$(U_BOOT_VERSION)" "$${localvers}" ; \
+		) > [email protected]

IMO, PLAIN_VERSION isn't descriptive enough (should really be called VERSION..?). How about going with something like:

#define U_BOOT_STR "U-Boot"
#define U_BOOT_VERSION U_BOOT_STR " %s%s"...

and then

tools/mkimage.c main (int argc, char **argv) · line 252

Quoted 3 lines
tools/mkimage.c +3 -0
+			case 'V':
+				printf("mkimage version %s\n", PLAIN_VERSION);
+				exit(EXIT_SUCCESS);

&U_BOOT_VERSION[sizeof(U_BOOT_STR)]

(the - 1 is not necessary since we want to include the ' ')

this maintains consistency and the fact that the mkimage version is directly tied to it's parent project, U-Boot's, version number.

Kim

author
#15 lore raw

PATCH v4 1/1 mkimage: add "-V" option to print version information

Dear Kim Phillips,

Quoted 13 lines · In message <[email protected]> you wrote:

Makefile $(U_BOOT_ONENAND): $(ONENAND_IPL) $(obj)u-boot.bin · line 422

Quoted 9 lines
Makefile +6 -2
-		@( printf '#define U_BOOT_VERSION "U-Boot %s%s"\n' "$(U_BOOT_VERSION)" \
-		 '$(shell $(TOPDIR)/tools/setlocalversion $(TOPDIR))' ) > [email protected]
+		@( localvers='$(shell $(TOPDIR)/tools/setlocalversion $(TOPDIR))' ; \
+		   printf '#define PLAIN_VERSION "%s%s"\n' \
+		   	"$(U_BOOT_VERSION)" "$${localvers}" ; \
+		   printf '#define U_BOOT_VERSION "U-Boot %s%s"\n' \
+		   	"$(U_BOOT_VERSION)" "$${localvers}" ; \
+		) > [email protected]

IMO, PLAIN_VERSION isn't descriptive enough (should really be called VERSION..?). How about going with something like:

Quoted 2 lines

#define U_BOOT_STR "U-Boot"
#define U_BOOT_VERSION U_BOOT_STR " %s%s"...

No - not unless you guarantee that this syntax is compatible with all assemblers that may be used to build U-Boot.

Quoted 9 lines

and then

tools/mkimage.c main (int argc, char **argv) · line 252

Quoted 3 lines
tools/mkimage.c +3 -0
+			case 'V':
+				printf("mkimage version %s\n", PLAIN_VERSION);
+				exit(EXIT_SUCCESS);

&U_BOOT_VERSION[sizeof(U_BOOT_STR)]

(the - 1 is not necessary since we want to include the ' ')

No again. This is, um, ugly, and completely unnecessary.

Best regards,

Wolfgang Denk

Signature · 6 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Man is the best computer we can put aboard a spacecraft ...  and  the
only one that can be mass produced with unskilled labor.
                                                  - Wernher von Braun
#16 lore raw

PATCH v4 1/1 mkimage: add "-V" option to print version information

Quoted 16 lines · On Sun, 13 Feb 2011 00:35:08 +0100 Wolfgang Denk <[email protected]> wrote:

Dear Kim Phillips,

Quoted 13 lines · In message <[email protected]> you wrote:

Makefile $(U_BOOT_ONENAND): $(ONENAND_IPL) $(obj)u-boot.bin · line 422

Quoted 9 lines
Makefile +6 -2
-		@( printf '#define U_BOOT_VERSION "U-Boot %s%s"\n' "$(U_BOOT_VERSION)" \
-		 '$(shell $(TOPDIR)/tools/setlocalversion $(TOPDIR))' ) > [email protected]
+		@( localvers='$(shell $(TOPDIR)/tools/setlocalversion $(TOPDIR))' ; \
+		   printf '#define PLAIN_VERSION "%s%s"\n' \
+		   	"$(U_BOOT_VERSION)" "$${localvers}" ; \
+		   printf '#define U_BOOT_VERSION "U-Boot %s%s"\n' \
+		   	"$(U_BOOT_VERSION)" "$${localvers}" ; \
+		) > [email protected]

IMO, PLAIN_VERSION isn't descriptive enough (should really be called VERSION..?). How about going with something like:

Quoted 5 lines
Quoted 2 lines

#define U_BOOT_STR "U-Boot"
#define U_BOOT_VERSION U_BOOT_STR " %s%s"...

No - not unless you guarantee that this syntax is compatible with all assemblers that may be used to build U-Boot.

I cannot do that, but, ok, not such a big deal:

/* prefix lengths must match */
#define U_BOOT_STR "U-Boot"
#define U_BOOT_VERSION "U-Boot %s%s"...

Quoted 9 lines
Quoted 7 lines

tools/mkimage.c main (int argc, char **argv) · line 252

Quoted 3 lines
tools/mkimage.c +3 -0
+			case 'V':
+				printf("mkimage version %s\n", PLAIN_VERSION);
+				exit(EXIT_SUCCESS);

&U_BOOT_VERSION[sizeof(U_BOOT_STR)]

(the - 1 is not necessary since we want to include the ' ')

No again. This is, um, ugly, and completely unnecessary.

that's a matter of personal taste, but IMO it's better than PLAIN_VERSION (what's that? - VERSION_NUMBERONLY would be way more descriptive).

If it's the &..[..] that's not appealing, feel free to do as the '+ 7' code but as '+ sizeof(...)' (to maintain a not-so-completely unnecessary clarity & consistency..but that's my opinion).

Kim

author Applied
#17 lore raw

PATCH v4 1/1 mkimage: add "-V" option to print version information

In reply to Wolfgang Denk

Dear Wolfgang Denk,

Quoted 9 lines · In message <[email protected]> you wrote:
Signed-off-by Wolfgang Denk <[email protected]>

---
v2: fix missing argument to printf() call.
v3: explain the magic "+ 7" offset into the version string
v3: avoid offset into U_BOOT_VERSION string completely and define
a new PLAIN_VERSION variable instead; this has the benefit that it can be used in other places as well where such version information might be needed (fw_{set,print}env etc. comes to mind).

Quoted 3 lines
 Makefile        |    8 ++++++--
 tools/mkimage.c |    6 ++++++
 2 files changed, 12 insertions(+), 2 deletions(-)

Applied.

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If something is different, it's either better or worse,  and  usually
both.                                                    - Larry Wall