* [U-Boot] [PATCH v2] mkimage: add "-V" option to print version information
2011-02-11 8:56 [U-Boot] [PATCH] mkimage: add "-V" option to print version information Wolfgang Denk
@ 2011-02-11 12:22 ` Wolfgang Denk
2011-02-11 21:55 ` Albert ARIBAUD
2011-02-11 21:52 ` [U-Boot] [PATCH] " Albert ARIBAUD
` (2 subsequent siblings)
3 siblings, 1 reply; 18+ messages in thread
From: Wolfgang Denk @ 2011-02-11 12:22 UTC (permalink / raw)
To: u-boot
Signed-off-by: Wolfgang Denk <wd@denx.de>
---
v2: fix missing argument to printf() call.
tools/mkimage.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/tools/mkimage.c b/tools/mkimage.c
index f5859d7..127be57 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -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);
}
--
1.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [U-Boot] [PATCH v2] mkimage: add "-V" option to print version information
2011-02-11 12:22 ` [U-Boot] [PATCH v2] " Wolfgang Denk
@ 2011-02-11 21:55 ` Albert ARIBAUD
2011-02-11 22:35 ` Wolfgang Denk
0 siblings, 1 reply; 18+ messages in thread
From: Albert ARIBAUD @ 2011-02-11 21:55 UTC (permalink / raw)
To: u-boot
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 :
> Signed-off-by: Wolfgang Denk<wd@denx.de>
> ---
> v2: fix missing argument to printf() call.
>
> tools/mkimage.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index f5859d7..127be57 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -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);
If that magic number 7 (and the addition, as well) has any reason, it
should at least be explained in a short comment.
> + 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,
--
Albert.
^ permalink raw reply [flat|nested] 18+ messages in thread* [U-Boot] [PATCH v2] mkimage: add "-V" option to print version information
2011-02-11 21:55 ` Albert ARIBAUD
@ 2011-02-11 22:35 ` Wolfgang Denk
0 siblings, 0 replies; 18+ messages in thread
From: Wolfgang Denk @ 2011-02-11 22:35 UTC (permalink / raw)
To: u-boot
Dear Albert ARIBAUD,
In message <4D55B03A.9030602@free.fr> you wrote:
>
> Please ignore my previous post on V1, I had not seen V2. My comment
> holds, though:
...
> > + 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
--
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH] mkimage: add "-V" option to print version information
2011-02-11 8:56 [U-Boot] [PATCH] mkimage: add "-V" option to print version information Wolfgang Denk
2011-02-11 12:22 ` [U-Boot] [PATCH v2] " Wolfgang Denk
@ 2011-02-11 21:52 ` Albert ARIBAUD
2011-02-11 22:41 ` [U-Boot] [PATCH v3] " Wolfgang Denk
2011-02-12 9:37 ` [U-Boot] [PATCH v4] " Wolfgang Denk
3 siblings, 0 replies; 18+ messages in thread
From: Albert ARIBAUD @ 2011-02-11 21:52 UTC (permalink / raw)
To: u-boot
Hi Wolfgang,
Le 11/02/2011 09:56, Wolfgang Denk a ?crit :
> Signed-off-by: Wolfgang Denk<wd@denx.de>
> ---
> tools/mkimage.c | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index f5859d7..788484d 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -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);
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.
> + 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,
--
Albert.
^ permalink raw reply [flat|nested] 18+ messages in thread* [U-Boot] [PATCH v3] mkimage: add "-V" option to print version information
2011-02-11 8:56 [U-Boot] [PATCH] mkimage: add "-V" option to print version information Wolfgang Denk
2011-02-11 12:22 ` [U-Boot] [PATCH v2] " Wolfgang Denk
2011-02-11 21:52 ` [U-Boot] [PATCH] " Albert ARIBAUD
@ 2011-02-11 22:41 ` Wolfgang Denk
2011-02-11 23:11 ` Kim Phillips
2011-02-12 9:37 ` [U-Boot] [PATCH v4] " Wolfgang Denk
3 siblings, 1 reply; 18+ messages in thread
From: Wolfgang Denk @ 2011-02-11 22:41 UTC (permalink / raw)
To: u-boot
Signed-off-by: Wolfgang Denk <wd@denx.de>
---
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(-)
diff --git a/tools/mkimage.c b/tools/mkimage.c
index f5859d7..febb536 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -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);
}
--
1.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [U-Boot] [PATCH v3] mkimage: add "-V" option to print version information
2011-02-11 22:41 ` [U-Boot] [PATCH v3] " Wolfgang Denk
@ 2011-02-11 23:11 ` Kim Phillips
2011-02-12 6:47 ` Albert ARIBAUD
2011-02-12 9:37 ` Wolfgang Denk
0 siblings, 2 replies; 18+ messages in thread
From: Kim Phillips @ 2011-02-11 23:11 UTC (permalink / raw)
To: u-boot
On Fri, 11 Feb 2011 23:41:43 +0100
Wolfgang Denk <wd@denx.de> wrote:
> + 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
^ permalink raw reply [flat|nested] 18+ messages in thread* [U-Boot] [PATCH v3] mkimage: add "-V" option to print version information
2011-02-11 23:11 ` Kim Phillips
@ 2011-02-12 6:47 ` Albert ARIBAUD
2011-02-12 9:29 ` Wolfgang Denk
2011-02-12 22:04 ` Kim Phillips
2011-02-12 9:37 ` Wolfgang Denk
1 sibling, 2 replies; 18+ messages in thread
From: Albert ARIBAUD @ 2011-02-12 6:47 UTC (permalink / raw)
To: u-boot
Le 12/02/2011 00:11, Kim Phillips a ?crit :
> On Fri, 11 Feb 2011 23:41:43 +0100
> Wolfgang Denk<wd@denx.de> wrote:
>
>> + 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.
> 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.
> Kim
Amicalement,
--
Albert.
^ permalink raw reply [flat|nested] 18+ messages in thread* [U-Boot] [PATCH v3] mkimage: add "-V" option to print version information
2011-02-12 6:47 ` Albert ARIBAUD
@ 2011-02-12 9:29 ` Wolfgang Denk
2011-02-12 22:04 ` Kim Phillips
1 sibling, 0 replies; 18+ messages in thread
From: Wolfgang Denk @ 2011-02-12 9:29 UTC (permalink / raw)
To: u-boot
Dear Albert ARIBAUD,
In message <4D562D0A.9060800@free.fr> 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
--
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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH v3] mkimage: add "-V" option to print version information
2011-02-12 6:47 ` Albert ARIBAUD
2011-02-12 9:29 ` Wolfgang Denk
@ 2011-02-12 22:04 ` Kim Phillips
1 sibling, 0 replies; 18+ messages in thread
From: Kim Phillips @ 2011-02-12 22:04 UTC (permalink / raw)
To: u-boot
On Sat, 12 Feb 2011 07:47:38 +0100
Albert ARIBAUD <albert.aribaud@free.fr> wrote:
> Le 12/02/2011 00:11, Kim Phillips a ?crit :
> > 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.
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH v3] mkimage: add "-V" option to print version information
2011-02-11 23:11 ` Kim Phillips
2011-02-12 6:47 ` Albert ARIBAUD
@ 2011-02-12 9:37 ` Wolfgang Denk
2011-02-12 22:17 ` Kim Phillips
1 sibling, 1 reply; 18+ messages in thread
From: Wolfgang Denk @ 2011-02-12 9:37 UTC (permalink / raw)
To: u-boot
Dear Kim Phillips,
In message <20110211171117.b9b05b01.kim.phillips@freescale.com> you wrote:
> On Fri, 11 Feb 2011 23:41:43 +0100
> Wolfgang Denk <wd@denx.de> wrote:
>
> > + 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 ")]
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
--
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."
^ permalink raw reply [flat|nested] 18+ messages in thread* [U-Boot] [PATCH v3] mkimage: add "-V" option to print version information
2011-02-12 9:37 ` Wolfgang Denk
@ 2011-02-12 22:17 ` Kim Phillips
2011-02-12 23:31 ` Wolfgang Denk
0 siblings, 1 reply; 18+ messages in thread
From: Kim Phillips @ 2011-02-12 22:17 UTC (permalink / raw)
To: u-boot
On Sat, 12 Feb 2011 10:37:16 +0100
Wolfgang Denk <wd@denx.de> wrote:
> Dear Kim Phillips,
>
> In message <20110211171117.b9b05b01.kim.phillips@freescale.com> you wrote:
> > On Fri, 11 Feb 2011 23:41:43 +0100
> > Wolfgang Denk <wd@denx.de> wrote:
> >
> > > + 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 ")]
>
> 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
^ permalink raw reply [flat|nested] 18+ messages in thread* [U-Boot] [PATCH v3] mkimage: add "-V" option to print version information
2011-02-12 22:17 ` Kim Phillips
@ 2011-02-12 23:31 ` Wolfgang Denk
0 siblings, 0 replies; 18+ messages in thread
From: Wolfgang Denk @ 2011-02-12 23:31 UTC (permalink / raw)
To: u-boot
Dear Kim Phillips,
In message <20110212161715.111c368f.kim.phillips@freescale.com> you wrote:
>
> > > I'd have done it without magic nor comments as
...
> > 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
--
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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH v4] mkimage: add "-V" option to print version information
2011-02-11 8:56 [U-Boot] [PATCH] mkimage: add "-V" option to print version information Wolfgang Denk
` (2 preceding siblings ...)
2011-02-11 22:41 ` [U-Boot] [PATCH v3] " Wolfgang Denk
@ 2011-02-12 9:37 ` Wolfgang Denk
2011-02-12 23:13 ` Kim Phillips
2011-04-12 20:37 ` Wolfgang Denk
3 siblings, 2 replies; 18+ messages in thread
From: Wolfgang Denk @ 2011-02-12 9:37 UTC (permalink / raw)
To: u-boot
Signed-off-by: Wolfgang Denk <wd@denx.de>
---
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(-)
diff --git a/Makefile b/Makefile
index 05b404d..8721b59 100644
--- a/Makefile
+++ b/Makefile
@@ -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))' ) > $@.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
@( printf '#define CC_VERSION_STRING "%s"\n' \
'$(shell $(CC) --version | head -n 1)' )>> $@.tmp
@( printf '#define LD_VERSION_STRING "%s"\n' \
diff --git a/tools/mkimage.c b/tools/mkimage.c
index f5859d7..60f7263 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -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);
}
--
1.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [U-Boot] [PATCH v4] mkimage: add "-V" option to print version information
2011-02-12 9:37 ` [U-Boot] [PATCH v4] " Wolfgang Denk
@ 2011-02-12 23:13 ` Kim Phillips
2011-02-12 23:35 ` Wolfgang Denk
2011-04-12 20:37 ` Wolfgang Denk
1 sibling, 1 reply; 18+ messages in thread
From: Kim Phillips @ 2011-02-12 23:13 UTC (permalink / raw)
To: u-boot
On Sat, 12 Feb 2011 10:37:11 +0100
Wolfgang Denk <wd@denx.de> 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"...
and then
> + 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
^ permalink raw reply [flat|nested] 18+ messages in thread* [U-Boot] [PATCH v4] mkimage: add "-V" option to print version information
2011-02-12 23:13 ` Kim Phillips
@ 2011-02-12 23:35 ` Wolfgang Denk
2011-02-13 0:08 ` Kim Phillips
0 siblings, 1 reply; 18+ messages in thread
From: Wolfgang Denk @ 2011-02-12 23:35 UTC (permalink / raw)
To: u-boot
Dear Kim Phillips,
In message <20110212171349.f0f5d472.kim.phillips@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.
> and then
>
> > + 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
--
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
^ permalink raw reply [flat|nested] 18+ messages in thread* [U-Boot] [PATCH v4] mkimage: add "-V" option to print version information
2011-02-12 23:35 ` Wolfgang Denk
@ 2011-02-13 0:08 ` Kim Phillips
0 siblings, 0 replies; 18+ messages in thread
From: Kim Phillips @ 2011-02-13 0:08 UTC (permalink / raw)
To: u-boot
On Sun, 13 Feb 2011 00:35:08 +0100
Wolfgang Denk <wd@denx.de> wrote:
> Dear Kim Phillips,
>
> In message <20110212171349.f0f5d472.kim.phillips@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
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH v4] mkimage: add "-V" option to print version information
2011-02-12 9:37 ` [U-Boot] [PATCH v4] " Wolfgang Denk
2011-02-12 23:13 ` Kim Phillips
@ 2011-04-12 20:37 ` Wolfgang Denk
1 sibling, 0 replies; 18+ messages in thread
From: Wolfgang Denk @ 2011-04-12 20:37 UTC (permalink / raw)
To: u-boot
Dear Wolfgang Denk,
In message <1297503431-19435-1-git-send-email-wd@denx.de> you wrote:
> Signed-off-by: Wolfgang Denk <wd@denx.de>
> ---
> 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(-)
Applied.
Best regards,
Wolfgang Denk
--
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
^ permalink raw reply [flat|nested] 18+ messages in thread