[U-Boot] [PATCH v4] mkimage: add "-V" option to print version information

[Kim Phillips]

On Sun, 13 Feb 2011 00:35:08 +0100
Wolfgang Denk <wd at denx.de> wrote:

> Dear Kim Phillips,
> 
> In message <20110212171349.f0f5d472.kim.phillips at freescale.com> you wrote:
> > 
> > > -		@( printf '#define U_BOOT_VERSION "U-Boot %s%s"\n' "$(U_BOOT_VERSION)" \
> > > -		 '$(shell $(TOPDIR)/tools/setlocalversion $(TOPDIR))' ) > $@.tmp
> > > +		@( 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}" ; \
> > > +		) > $@.tmp
> > 
> > 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"...
> 
> 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"...

> > > +			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