new tool mkenvimage: generates an env image from an arbitrary config file
PATCH 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
This tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environnment image, ready to be flashed.
--- tools/Makefile | 5 ++ tools/mkenvimage.c | 157 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 162 insertions(+), 0 deletions(-) create mode 100644 tools/mkenvimage.c
tools/Makefile +5 -0
@@ -69,6 +69,7 @@ BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX) BIN_FILES-y += mkimage$(SFX) BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX) BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX) +BIN_FILES-y += mkenvimage$(SFX) # Source files which exist outside the tools directory EXT_OBJ_FILES-$(CONFIG_BUILD_ENVCRC) += common/env_embedded.o
@@ -93,6 +94,7 @@ OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o NOPED_OBJ_FILES-y += os_support.o OBJ_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1.o NOPED_OBJ_FILES-y += ublimage.o +NOPED_OBJ_FILES-y += mkenvimage.o # Don't build by default #ifeq ($(ARCH),ppc)
@@ -171,6 +173,9 @@ $(obj)bmp_logo$(SFX): $(obj)bmp_logo.o $(obj)envcrc$(SFX): $(obj)crc32.o $(obj)env_embedded.o $(obj)envcrc.o $(obj)sha1.o $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ +$(obj)mkenvimage$(SFX): $(obj)crc32.o $(obj)mkenvimage.o + $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ + $(obj)gen_eth_addr$(SFX): $(obj)gen_eth_addr.o $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ $(HOSTSTRIP) $@
tools/mkenvimage.c new file +157 -0
157 lines not on this page · raw message
Signature · 1 line
1.7.0.4
PATCH 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
Hello,
Le Fri, 5 Aug 2011 16:49:58 +0200,
David Wagner <[email protected]> a ?crit :
Quoted 2 lines
This tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environnment image, ready to be flashed.
Nice tool. I'm currently using a crappy shell-based equivalent of this, but it'd be a lot better to have a clean and nice version integrated in U-Boot. However, see some comments below:
tools/mkenvimage.c line 49
Quoted 1 line
tools/mkenvimage.c new file +1
-0+"\t-r : the environnment is redundand\n"
"the environment has two copies in flash" might be clearer that this "redundand" word maybe.
tools/mkenvimage.c line 65
Quoted 12 lines
tools/mkenvimage.c new file +12
-0+static int make_binary_config (FILE * txt_file,unsigned char *envptr,int envsize) +{ +int i; +int ret; + + ret =fread (envptr, envsize,1 , txt_file); +for (i =0 ; i < envsize ; i++) +if (envptr[i] =='\n' ) + envptr[i] ='\0' ; + +return 0 ; +}
The name of the function sounds a bit strange, and the function's job is really small. Maybe it should just be merged into the main function.
Quoted 8 lines
tools/mkenvimage.c new file +8
-0+int main (int argc,char **argv) +{ +uint32_t crc; +char *txt_filename =NULL , *bin_filename =NULL ; +FILE *txt_file, *bin_file; +unsigned char *dataptr, *envptr; +unsigned int envsize, datasize =0 ; +int bigendian =0 ;
tools/mkenvimage.c line 84
Quoted 10 lines
tools/mkenvimage.c new file +10
-0+int redundant =0 ; + +int option; +int ret =EXIT_SUCCESS ; + + opterr =0 ; + + +/* Parse the cmdline */ +while ((option =getopt (argc, argv,"s:o:rbh" )) !=-1 )
I'd prefer to have an opening brace here, even if there is technically a single statement inside the while() loop.
tools/mkenvimage.c line 89
Quoted 5 lines
tools/mkenvimage.c new file +5
-0+switch (option) + { +case 's' : + datasize =atoi (optarg); +break ;
It'd be nicer if sizes could be given in decimal *or* hexadecimal formats. 0x20000 is much easier to type than 131072.
tools/mkenvimage.c line 106
Quoted 3 lines
tools/mkenvimage.c new file +3
-0+default : +if (bin_filename) +free (bin_filename);
Don't try to do needless cleanup, and let the operating system do it for you. It's a short-lived program, I don't think it's worth worrying about cleaning-up things.
tools/mkenvimage.c line 113
Quoted 7 lines
tools/mkenvimage.c new file +7
-0+fprintf (stderr,"Wrong option\n" ); +usage (); +return EXIT_FAILURE ; + } + +if (datasize ==0 ) { +printf ("Please specify the size of the envrionnment partition.\n" );
^^^^^ typo
The message should probably be printed to stderr:
fprintf(stderr, "blabla\n");
tools/mkenvimage.c line 116
Quoted 3 lines
tools/mkenvimage.c new file +3
-0+usage (); + ret =EXIT_FAILURE ; + goto out;
just return EXIT_FAILURE, not need to clean up.
tools/mkenvimage.c line 123
Quoted 7 lines
tools/mkenvimage.c new file +7
-0+ } + + + txt_filename =strdup (argv[optind]); +if (!txt_filename) { + ret =ENOMEM ; + goto out;
no need to clean up.
tools/mkenvimage.c line 128
Quoted 5 lines
tools/mkenvimage.c new file +5
-0+ } + + txt_file =fopen (txt_filename,"r" ); +if (!txt_file) + goto out;
You should check whether the size of the environment given in the file doesn't exceed the size of the environment passed on the command line.
tools/mkenvimage.c line 132
Quoted 4 lines
tools/mkenvimage.c new file +4
-0+/* Read the raw configuration file and transform it */ + dataptr =calloc (datasize,1 ); +if (!dataptr) + goto out;
No need to clean up.
tools/mkenvimage.c line 141
Quoted 8 lines
tools/mkenvimage.c new file +8
-0+ envsize = datasize - (CRC_SIZE + redundant); + envptr = dataptr +CRC_SIZE + redundant; + + ret =make_binary_config (txt_file, envptr, envsize); + ret =fclose (txt_file); + + crc =crc32 (0 , envptr, envsize); +printf ("crc: 0x%08X\n" , crc);
I don't think printing the CRC is useful, just drop.
tools/mkenvimage.c line 143
Quoted 1 line
tools/mkenvimage.c new file +1
-0+ *((uint32_t *) dataptr) = bigendian ?htobe32 (crc) :htole32 (crc);
I think it'd be better to have :
struct env_normal {
uint32_t crc;
char data[0];
}struct env_redund {
uint32_t crc;
char flags;
char data[0];
}rather than this cast.
tools/mkenvimage.c line 147
Quoted 3 lines
tools/mkenvimage.c new file +3
-0+ bin_file =fopen (bin_filename,"w" ); +if (fwrite (dataptr,1 , datasize, bin_file) != datasize) +fprintf (stderr,"fwrite() failed: %s\n" ,strerror (errno));
Missing exit with error here.
tools/mkenvimage.c line 156
Quoted 8 lines
tools/mkenvimage.c new file +8
-0+ ret =fclose (bin_file); + +out: +if (txt_filename) +free (txt_filename); +if (bin_filename) +free (bin_filename); +return ret;
No need to do useless clean up.
Regards,
Thomas
Signature · 4 lines
Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH v2 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
In reply to David WagnerThis tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environnment image, ready to be flashed.
---
Hi Thomas,
This second version addresses all your comments except - as we discussed - the cast vs. struct part.
tools/Makefile | 5 ++ tools/mkenvimage.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 192 insertions(+), 0 deletions(-) create mode 100644 tools/mkenvimage.c
tools/Makefile +5 -0
@@ -69,6 +69,7 @@ BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX) BIN_FILES-y += mkimage$(SFX) BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX) BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX) +BIN_FILES-y += mkenvimage$(SFX) # Source files which exist outside the tools directory EXT_OBJ_FILES-$(CONFIG_BUILD_ENVCRC) += common/env_embedded.o
@@ -93,6 +94,7 @@ OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o NOPED_OBJ_FILES-y += os_support.o OBJ_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1.o NOPED_OBJ_FILES-y += ublimage.o +NOPED_OBJ_FILES-y += mkenvimage.o # Don't build by default #ifeq ($(ARCH),ppc)
@@ -171,6 +173,9 @@ $(obj)bmp_logo$(SFX): $(obj)bmp_logo.o $(obj)envcrc$(SFX): $(obj)crc32.o $(obj)env_embedded.o $(obj)envcrc.o $(obj)sha1.o $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ +$(obj)mkenvimage$(SFX): $(obj)crc32.o $(obj)mkenvimage.o + $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ + $(obj)gen_eth_addr$(SFX): $(obj)gen_eth_addr.o $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ $(HOSTSTRIP) $@
tools/mkenvimage.c new file +187 -0
187 lines not on this page · raw message
Signature · 1 line
1.7.0.4
PATCH v2 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
Quoted 1 line · David Wagner
+#include <
endian.h>
this is not portable. include compiler.h and use the already existing uswap/cpu_to/to_cpu/etc... helpers.
tools/mkenvimage.c
Quoted 3 lines
tools/mkenvimage.c new file +3
-0+static void usage (void ) +{ + ? ? ? printf("envcrc [-h] [-r] [-b] -s <envrionnment partition size> -o <output> "
funny, "envcrc" isnt what the filename actually is ...
typo with "environment"
seems like this should also take a padding byte so people can use a more sensible 0xff rather than 0x00
Review
Quoted 3 lines
+ ? ? ? ? ? ? ?"\t-b : the target is big endian (default is little endian)\n");
(quoted diff) +2
-0+ +}
kill useless newline before brace
Quoted 1 line
+ ? ? ? opterr = 0;
unnecessary ... punt it
Quoted 2 lines
+ ? ? ? ? ? ? ? switch (option)
+ ? ? ? ? ? ? ? {
cuddle the brace up
Quoted 2 lines
+ ? ? ? /* Check the configuration file ...*/
+ ? ? ?txt_filename=strdup(argv[optind]);
no need to strdup this. argv isnt going anywhere.
Quoted 6 lines
+ ? ? ? ret =
stat(txt_filename, &txt_file_stat);
+ ? ? ? if (ret == -1) {
+ ? ? ? ? ? ? ? fprintf(stderr, "Can'tstat()on configuration file: %s",
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? strerror(errno));
+ ? ? ? ? ? ? ? returnEXIT_FAILURE;
+ ? ? ? }
Quoted 8 lines
+ ? ? ? if (txt_file_stat.st_size > envsize) {
+ ? ? ? ? ? ? ? fprintf(stderr, "The configuration file is larger than the "
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "envrionnment partition size\n");
+ ? ? ? ? ? ? ? returnEXIT_FAILURE;
+ ? ? ? }
+
+ ? ? ? /* ... and open it. */
+ ? ? ?txt_file=fopen(txt_filename, "r");
fopen() it, then fstat() the fileno(txt_file) to avoid possible race conditions. also, use "rb".
Quoted 2 lines
+ ? ? ? /* Read the raw configuration file and transform it */
+ ? ? ? ret =fread(envptr, txt_file_stat.st_size, 1, txt_file);
you've swapped size and nemb args
Quoted 3 lines
+ ? ? ? for (i = 0 ; i < envsize ; i++)
+ ? ? ? ? ? ? ? if (envptr[i] == '\n')
+ ? ? ? ? ? ? ? ? ? ? ? envptr[i] = '\0';
you could use memchr() and a while loop instead ... but probably not worth the effort
Quoted 1 line
+ ? ? ? ret =
fclose(txt_file);
you could fclose() before the envptr conversion since you're done with the file at that point
Quoted 1 line
+ ? ? ? *((uint32_t*) dataptr) = bigendian ? htobe32(crc) : htole32(crc);
create a local uint32_t variable to set, then use memcpy to copy it over to dataptr
Quoted 1 line
+ ? ? ?
bin_file=fopen(bin_filename, "w");
"wb"
Quoted 1 line
+ ? ? ? if (
fwrite(dataptr, 1, datasize, bin_file)!= datasize) {
funny enough, you got the size/nemb args in the right order here ;) -mike
PATCH v2 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
Hi,
Quoted 9 lines · Mike Frysinger
Quoted 1 line · David Wagner
+#include <
endian.h>this is not portable. include
compiler.hand use the already existing uswap/cpu_to/to_cpu/etc... helpers.tools/mkenvimage.c
Quoted 3 lines
tools/mkenvimage.c new file +3
-0+static void usage (void ) +{ + printf("envcrc [-h] [-r] [-b] -s <envrionnment partition
size> -o <output> "
Quoted 2 lines
funny, "envcrc" isnt what the filename actually is ...
ah, yes, it began as an attempt to extend envcrc but ended up fairly different.
Quoted 2 lines
seems like this should also take a padding byte so people can use a more sensible
0xffrather than0x00
Thomas told me that when padding with 0xff, his environment image wouldn't be taken into account anymore.
I'll add the option anyway, but do you know what could be happening ?
Also, I guess this makes necessary making sure there is a \0 after the last configuration line.
Quoted 4 lines
Quoted 1 line
+ if (
fwrite(dataptr, 1, datasize, bin_file)!= datasize) {funny enough, you got the size/nemb args in the right order here ;) -mike
I wasn't sure whether the proper way of doing it was "read/write 1 byte, N times" or "read/write the size of the file, 1 time". I probably changed my mine in between.
Thanks for reviewing ; David.
Signature · 4 lines
David Wagner, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH v2 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
Hi David,
Le 08/08/2011 10:16, David Wagner a ?crit :
Quoted 9 lines
Quoted 3 lines
tools/mkenvimage.c
Quoted 3 lines
tools/mkenvimage.c new file +3
-0+static void usage (void ) +{ + printf("envcrc [-h] [-r] [-b] -s<envrionnment partitionsize> -o<output> "
Quoted 2 lines
funny, "envcrc" isnt what the filename actually is ...
ah, yes, it began as an attempt to extend envcrc but ended up fairly different.
Maybe passing argv[0] to usage() would help deal with the current name of the executable?
Amicalement,
Signature · 1 line
Albert.
PATCH v2 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
Quoted 11 lines · Albert ARIBAUD
Hi David,
Le 08/08/2011 10:16, David Wagner a ?crit :
Quoted 7 lines
Quoted 3 lines
tools/mkenvimage.c
Quoted 3 lines
tools/mkenvimage.c new file +3
-0+static void usage (void ) +{ + printf("envcrc [-h] [-r] [-b] -s<envrionnment partitionsize> -o<output> "
Quoted 2 lines
funny, "envcrc" isnt what the filename actually is ...
Quoted 7 lines
Quoted 2 lines
ah, yes, it began as an attempt to extend envcrc but ended up fairly different.
Maybe passing argv[0] to
usage()would help deal with the current name of the executable?Amicalement,
Indeed, merci.
Signature · 4 lines
David Wagner, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH v2 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
In reply to David WagnerQuoted 7 lines · David Wagner
Quoted 2 lines · Mike Frysinger
seems like this should also take a padding byte so people can use a more sensible
0xffrather than0x00Thomas told me that when padding with
0xff, his environment image wouldn't be taken into account anymore.
I'll add the option anyway, but do you know what could be happening ?
sorry, but i dont know what you're referring to. perhaps you wrote out the padding before calculating the crc ? ive been using tools/envcrc --binary for a while now and it pads things with 0xff.
Quoted 2 lines
Also, I guess this makes necessary making sure there is a \0 after the last configuration line.
two to be exact ... one for the config opt, and one to tell u-boot that it's the end of the env.
Quoted 3 lines
I wasn't sure whether the proper way of doing it was "read/write 1 byte, N times" or "read/write the size of the file, 1 time". ?I probably changed my mine in between.
dont think of it in terms of bytes. in fact, forget about that completely. think of it in terms of "elements". in this case, the element is a "char" which means you want to read a bunch of "char" elements and the size of each is 1.
you could also do:fwrite(dataptr, sizeof(*dataptr), datasize, bin_file); then you dont need to think "how many bytes is one element". -mike
PATCH 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
In reply to David WagnerThis tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environnment image, ready to be flashed.
---
Hi Mike,
This 3rd version should address what you pointed out.
I had troubles testing it on my IGEP board: I couldn't compile from the official git (I think it's because of my toolchain ... a more recent toolchain might be able to compile but won't compile IGEP's uboot version ... weird stuff above my head involved). And the IGEP uboot seems broken wrt environnment in onenand:
when computing the CRC, it only takes the 4092 (4096 minus the header) bytes of the environment into account ...
So, someone else should probably test it.
David.
tools/Makefile | 5 ++ tools/mkenvimage.c | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 210 insertions(+), 0 deletions(-) create mode 100644 tools/mkenvimage.c
tools/Makefile +5 -0
@@ -69,6 +69,7 @@ BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX) BIN_FILES-y += mkimage$(SFX) BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX) BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX) +BIN_FILES-y += mkenvimage$(SFX) # Source files which exist outside the tools directory EXT_OBJ_FILES-$(CONFIG_BUILD_ENVCRC) += common/env_embedded.o
@@ -93,6 +94,7 @@ OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o NOPED_OBJ_FILES-y += os_support.o OBJ_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1.o NOPED_OBJ_FILES-y += ublimage.o +NOPED_OBJ_FILES-y += mkenvimage.o # Don't build by default #ifeq ($(ARCH),ppc)
@@ -171,6 +173,9 @@ $(obj)bmp_logo$(SFX): $(obj)bmp_logo.o $(obj)envcrc$(SFX): $(obj)crc32.o $(obj)env_embedded.o $(obj)envcrc.o $(obj)sha1.o $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ +$(obj)mkenvimage$(SFX): $(obj)crc32.o $(obj)mkenvimage.o + $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ + $(obj)gen_eth_addr$(SFX): $(obj)gen_eth_addr.o $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ $(HOSTSTRIP) $@
tools/mkenvimage.c new file +205 -0
205 lines not on this page · raw message
Signature · 1 line
1.7.0.4
PATCH 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
Quoted 1 line · On Tuesday, August 09, 2011 06:31:29 David Wagner wrote:
+extern
uint32_tcrc32 (uint32_t, const unsigned char *, unsigned int);
does including u-boot/crc.h work ?
Quoted 1 line
+static void
usage(char *exec_name)
const char *exec_name
tools/mkenvimage.c line 60
Quoted 1 line
tools/mkenvimage.c new file +1
-0+char *txt_filename =NULL , *bin_filename =NULL ;
mark them both const ?
tools/mkenvimage.c line 117
Quoted 1 line
tools/mkenvimage.c new file +1
-0+ dataptr =calloc (datasize,sizeof (*dataptr));
you zero-ed it out here ...
tools/mkenvimage.c line 129
Quoted 2 lines
tools/mkenvimage.c new file +2
-0+/* Pad the environnment with the padding byte */ +memset (envptr, padbyte, envsize);
... then manually filled it with a byte. just use malloc() instead of calloc() and avoid that zero.
tools/mkenvimage.c line 188
Quoted 1 line
tools/mkenvimage.c new file +1
-0+memcpy ((uint32_t *) dataptr, &targetendian_crc,sizeof (uint32_t ));
i dont think that cast is necessary. just punt it.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110821/d79e0d6e/attachment.pgp
PATCH 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
In reply to David WagnerDear David Wagner,
Quoted 2 lines · In message <1312885889-20222-1-git-send-email-david.wagner@free-electrons.com>…
This tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environnment image, ready to be flashed.
s/nnm/nm/
Quoted 7 lines
Signed-off-by David Wagner <[email protected]>---
Hi Mike,
This 3rd version should address what you pointed out.
If this is v3, then why don't you say so in the Subject: ???
Could you please explain which usage scenarios you have in mind for this tool? I would probably rather just load the text file you use as input, and run "env import -t" on it.
Checkpatch says:
total: 4 errors, 5 warnings, 228 lines checked
Please fix these.
In addition:
...
tools/mkenvimage.c line 109
Quoted 9 lines
tools/mkenvimage.c new file +9
-0+default : +fprintf (stderr,"Wrong option -%c\n" , option); +usage (argv[0 ]); +return EXIT_FAILURE ; + } + } + + +/* Check datasize and allocate the data */
Please only one blank line in cases like this.
tools/mkenvimage.c line 124
Quoted 2 lines
tools/mkenvimage.c new file +2
-0+/* envptr points to the beginning of the actual environment (after the +* crc and possible `redundant' bit */
inforrect multiline comment style.
tools/mkenvimage.c line 135
Quoted 5 lines
tools/mkenvimage.c new file +5
-0+/* Open the configuration file ... */ + txt_filename = argv[optind]; +if (!txt_filename) { +fprintf (stderr,"Can't strdup() the configuration filename\n" ); +return EXIT_FAILURE ;
Where is there any strdup() involved ?
tools/mkenvimage.c line 139
Quoted 3 lines
tools/mkenvimage.c new file +3
-0+ txt_file =fopen (txt_filename,"rb" ); +if (!txt_file) { +fprintf (stderr,"Can't open configuration file: %s\n" ,strerror (errno));
It would probably be very useful to also print the file name.
tools/mkenvimage.c line 143
Quoted 2 lines
tools/mkenvimage.c new file +2
-0+/* ... and check it */ + ret =fstat (fileno (txt_file), &txt_file_stat);
Why don't you simply use mmap() ?
tools/mkenvimage.c line 166
Quoted 3 lines
tools/mkenvimage.c new file +3
-0+for (i =0 ; i < txt_file_stat.st_size ; i++) +if (envptr[i] =='\n' ) + envptr[i] ='\0' ;
This is actually wrong. Envrionment variables can have embedded newlines.
tools/mkenvimage.c line 169
Quoted 3 lines
tools/mkenvimage.c new file +3
-0+/* +* Make sure there is a final '\0' (necessary if the padding byte isn't +* 0x00 or if there wasn't a newline at the end of the configuration
You did not take this into account in your file size check above, it seems.
tools/mkenvimage.c line 171
Quoted 2 lines
tools/mkenvimage.c new file +2
-0+* file) Also, don't add it if the configuration file size is exactly +* the size of the environnment.
The double '\0' termination is _always_ needed.
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 You can observe a lot just by watchin'. - Yogi Berra
PATCH v4 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
In reply to David WagnerThis tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environment image, ready to be flashed.
For now, it doesn't work properly if environment variables have embedded newlines.
---
changes since v3 ~~~~~~~~~~~~~~~~
* cosmetic changes (checkpatch, spelling, blank lines)
* fail with an error message if it's impossible to have two ending '\0'
@Wolgang:
What is the advantage of using mmap() here ? the file is read entirely and sequentially ; does it make much of a difference compared to fread() ?
PS: Until a proper way is found for replacing only newlines that separate two environment variables (and not the ones inside a variable), let's just warn that it isn't supported.
tools/Makefile | 5 + tools/mkenvimage.c | 219 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 224 insertions(+), 0 deletions(-) create mode 100644 tools/mkenvimage.c
tools/Makefile +5 -0
@@ -69,6 +69,7 @@ BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX) BIN_FILES-y += mkimage$(SFX) BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX) BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX) +BIN_FILES-y += mkenvimage$(SFX) # Source files which exist outside the tools directory EXT_OBJ_FILES-$(CONFIG_BUILD_ENVCRC) += common/env_embedded.o
@@ -94,6 +95,7 @@ OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o NOPED_OBJ_FILES-y += os_support.o OBJ_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1.o NOPED_OBJ_FILES-y += ublimage.o +NOPED_OBJ_FILES-y += mkenvimage.o # Don't build by default #ifeq ($(ARCH),ppc)
@@ -172,6 +174,9 @@ $(obj)bmp_logo$(SFX): $(obj)bmp_logo.o $(obj)envcrc$(SFX): $(obj)crc32.o $(obj)env_embedded.o $(obj)envcrc.o $(obj)sha1.o $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ +$(obj)mkenvimage$(SFX): $(obj)crc32.o $(obj)mkenvimage.o + $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ + $(obj)gen_eth_addr$(SFX): $(obj)gen_eth_addr.o $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ $(HOSTSTRIP) $@
tools/mkenvimage.c new file +219 -0
219 lines not on this page · raw message
Signature · 1 line
1.7.0.4
PATCH v4 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
Quoted 3 lines · On Monday, August 29, 2011 08:06:46 David Wagner wrote:
This tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environment image, ready to be flashed.
this one line desc is pretty succulent. i'd suggest adding it to the usage() so that people who run ./tools/mkenvimage -h get the idea immediately.
the rest looks pretty straightforward without actually compiling/testing :). thanks !
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110830/e8e45e42/attachment.pgp
PATCH v4 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
In reply to David WagnerDear David Wagner,
Quoted 5 lines · In message <[email protected]>…
This tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environment image, ready to be flashed.
For now, it doesn't work properly if environment variables have embedded newlines.
I think this should be added for compatibility with both the printenv output and the result of "env export -t"
Quoted 3 lines
@Wolgang:
What is the advantage of usingmmap()here ? the file is read entirely and sequentially ; does it make much of a difference compared tofread()?
It's easier to go back in the stream without allocating buffers that are at least as big as the file.
Quoted 3 lines
PS: Until a proper way is found for replacing only newlines that separate two environment variables (and not the ones inside a variable), let's just warn that it isn't supported.
What do you mean by "Until a proper way is found"? There is nothing to be found. Just have a look at the "env import" code which does exactly that. Alternatively, as you are only dealing with text format anyway, look if the character immediately preceeding the newline is a backslash:
=> setenv x 'line 1 > line 2' => printenv ... x=line 1\ line 2
tools/Makefile BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX) · line 72
Quoted 5 lines
tools/Makefile +1
-0@@ -69,6 +69,7 @@ BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX) BIN_FILES-y += mkimage$(SFX) BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX) BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX) +BIN_FILES-y += mkenvimage$(SFX)
Please keep list sorted.
tools/Makefile OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o · line 98
Quoted 7 lines
tools/Makefile +1
-0# Source files which exist outside the tools directory EXT_OBJ_FILES-$(CONFIG_BUILD_ENVCRC) += common/env_embedded.o@@ -94,6 +95,7 @@ OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o NOPED_OBJ_FILES-y += os_support.o OBJ_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1.o NOPED_OBJ_FILES-y += ublimage.o +NOPED_OBJ_FILES-y += mkenvimage.o
Ditto.
tools/Makefile $(obj)bmp_logo$(SFX): $(obj)bmp_logo.o · line 182
Quoted 12 lines
tools/Makefile +3
-0# Don't build by default #ifeq ($(ARCH),ppc)@@ -172,6 +174,9 @@ $(obj)bmp_logo$(SFX): $(obj)bmp_logo.o $(obj)envcrc$(SFX): $(obj)crc32.o $(obj)env_embedded.o $(obj)envcrc.o $(obj)sha1.o $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ +$(obj)mkenvimage$(SFX): $(obj)crc32.o $(obj)mkenvimage.o + $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ + $(obj)gen_eth_addr$(SFX): $(obj)gen_eth_addr.o $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ $(HOSTSTRIP) $@
Ditto.
...
tools/mkenvimage.c line 51
Quoted 10 lines
tools/mkenvimage.c new file +10
-0+static void usage (const char *exec_name) +{ +printf ("%s [-h] [-r] [-b] [-p <byte>] " +"-s <envrionment partition size> -o <output> <input file>\n" +"\n" +"\tThe input file is in format:\n" +"\t\tkey1=value1\n" +"\t\tkey2=value2\n" +"\t\t...\n" + "\t-r : the environment has two copies in flash\n"
Please s/two/multiple/ or s/two/more than one/ - especially on NAND we can have more than just 2 copies.
tools/mkenvimage.c line 112
Quoted 3 lines
tools/mkenvimage.c new file +3
-0+if (datasize ==0 ) { +fprintf (stderr, + "Please specify the size of the envrionnment "
s/envrionnment/environment/
Please fix globally (same error further down below).
tools/mkenvimage.c line 138
Quoted 5 lines
tools/mkenvimage.c new file +5
-0+/* Open the configuration file ... */ +if (optind >= argc) { +fprintf (stderr,"Please specify a configuration filename\n" ); +return EXIT_FAILURE ; + }
Why don;t you allow reading from stdin? It is good old Unix tradition that all commands can be used in pipes. Also, input file name '-' should select stdin.
Um... and why do you call it "configuration file"? It's not configuration data, it's plain input data, so rather call it "input file"
tools/mkenvimage.c line 144
Quoted 5 lines
tools/mkenvimage.c new file +5
-0+ txt_filename = argv[optind]; + txt_file =fopen (txt_filename,"rb" ); +if (!txt_file) { +fprintf (stderr,"Can't open configuration file \"%s\": %s\n" , + txt_filename,strerror (errno));
Here it's even better to omit the "configuration file " part completely.
tools/mkenvimage.c line 151
Quoted 6 lines
tools/mkenvimage.c new file +6
-0+ } +/* ... and check it */ + ret =fstat (fileno (txt_file), &txt_file_stat); +if (ret ==-1 ) { +fprintf (stderr,"Can't stat() on configuration file \"%s\": " +" %s\n" , txt_filename,strerror (errno));
Same here.
tools/mkenvimage.c line 160
Quoted 9 lines
tools/mkenvimage.c new file +9
-0+return EXIT_FAILURE ; + } +/* +* The right test to do is "=>" (not ">") because of the additionnal +* ending \0. See below. +*/ +if (txt_file_stat.st_size >= envsize) { +fprintf (stderr,"The configuration file is larger than the " +"envrionnment partition size\n" );
See note above.
tools/mkenvimage.c line 174
Quoted 3 lines
tools/mkenvimage.c new file +3
-0+for (i =0 ; i < txt_file_stat.st_size ; i++) +if (envptr[i] =='\n' ) + envptr[i] ='\0' ;
This needs braces.
tools/mkenvimage.c line 178
Quoted 4 lines
tools/mkenvimage.c new file +4
-0+/* +* Make sure there is a final '\0' (necessary if the padding byte isn't +* 0x00 or if there wasn't a newline at the end of the configuration +* file)
The double '\0' termination is _always_ necessary. Please avoid constructing special cases where there aren't any.
tools/mkenvimage.c line 192
Quoted 13 lines
tools/mkenvimage.c new file +13
-0+* And do it again on the next byte to mark the end of the environment. +*/ +if (i < envsize && envptr[i-1 ] !='\0' ) { + envptr[i++] ='\0' ; +/* +* The text file doesn't have an ending newline. We need to +* check the env size again to make sure we have room for two \0 +*/ +if (i >= envsize) { +fprintf (stderr,"The configuration is too large for the" +" target environment storage\n" ); +return EXIT_FAILURE ; + }
If you test this here, you can remove the test above.
tools/mkenvimage.c line 216
Quoted 13 lines
tools/mkenvimage.c new file +13
-0+ bin_file =fopen (bin_filename,"wb" ); +if (!bin_file) { +fprintf (stderr,"Can't open output file \"%s\": %s\n" , + bin_filename,strerror (errno)); +return EXIT_FAILURE ; + } + +if (fwrite (dataptr,sizeof (*dataptr), datasize, bin_file) != datasize) { +fprintf (stderr,"fwrite() failed: %s\n" ,strerror (errno)); +return EXIT_FAILURE ; + } + + ret =fclose (bin_file);
Is there any good reason for using stdio functions (fopen(), fread(), fwrite(), fclose()) instead of plain system calls (open(), read(), write(), close()) ?
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 What's the sound a name makes when it's dropped?
PATCH v5 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
In reply to David WagnerThis tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environment image, ready to be flashed.
use case: flash the environment with an external tool
---
changes since v4 ~~~~~~~~~~~~~~~~
- keep the Makefiles somewhat sorted (I'm not sure how it is sorted but since "mkenvimage" is alphabetically before "mkimage", I put the former just before the latter).
- replace f{open, read, write, ...} with {open, read, write, ...} and remove the now unused stdlib.h include
- tweak usage()
- typos
- properly handle embedded newlines, comments and empty lines at the beginning of the input file. This introduced a second buffer ('filebuf') (see the big for loop): When we encouter a newline after a backslash, without this buffer, we would have needed to shift all envptr one byte backward to overwrite the preceeding backslash. Introducing this second buffer looked like a simpler alternative.
- correct the way how 2 \0 are put at the end of the environment (@Wolfgang:
the second "ep >= envsize" check is there to handle the corner case where the env target partition would be, say, 32 bytes long and the input file is 31 byte long but doesn't have an ending newline)
PS: as the input file is probably always pretty small and we read the file sequentially, we don't go back in the stream, we didn't find much advantage over read()
tools/Makefile | 5 + tools/mkenvimage.c | 263 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 268 insertions(+), 0 deletions(-) create mode 100644 tools/mkenvimage.c
tools/Makefile +5 -0
@@ -66,6 +66,7 @@ BIN_FILES-$(CONFIG_BUILD_ENVCRC) += envcrc$(SFX) BIN_FILES-$(CONFIG_CMD_NET) += gen_eth_addr$(SFX) BIN_FILES-$(CONFIG_CMD_LOADS) += img2srec$(SFX) BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX) +BIN_FILES-y += mkenvimage$(SFX) BIN_FILES-y += mkimage$(SFX) BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX) BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
@@ -89,6 +90,7 @@ OBJ_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes.o NOPED_OBJ_FILES-y += kwbimage.o NOPED_OBJ_FILES-y += imximage.o NOPED_OBJ_FILES-y += omapimage.o +NOPED_OBJ_FILES-y += mkenvimage.o NOPED_OBJ_FILES-y += mkimage.o OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o NOPED_OBJ_FILES-y += os_support.o
@@ -184,6 +186,9 @@ $(obj)xway-swap-bytes$(SFX): $(obj)xway-swap-bytes.o $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ $(HOSTSTRIP) $@ +$(obj)mkenvimage$(SFX): $(obj)crc32.o $(obj)mkenvimage.o + $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ + $(obj)mkimage$(SFX): $(obj)crc32.o \ $(obj)default_image.o \ $(obj)fit_image.o \
tools/mkenvimage.c new file +263 -0
263 lines not on this page · raw message
Signature · 1 line
1.7.0.4
PATCH v5 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
Quoted 25 lines · On Thursday, September 01, 2011 11:57:09 David Wagner wrote:
tools/mkenvimage.c new file +25
-0+ txt_filename = argv[optind]; +if (strcmp (txt_filename,"-" ) ==0 ) { + txt_fd =STDIN_FILENO ; + }else { + txt_fd =open (txt_filename,O_RDONLY ); +if (txt_fd ==-1 ) { +fprintf (stderr,"Can't open \"%s\": %s\n" , + txt_filename,strerror (errno)); +return EXIT_FAILURE ; + } + } +/* ... and check it */ + ret =fstat (txt_fd, &txt_file_stat); +if (ret ==-1 ) { +fprintf (stderr,"Can't stat() on \"%s\": " +"%s\n" , txt_filename,strerror (errno)); +return EXIT_FAILURE ; + } +/* +* The right test to do is "=>" (not ">") because of the additionnal +* ending \0. See below. +*/ +if (txt_file_stat.st_size >= envsize) { +fprintf (stderr,"The input file is larger than the " +"envrionnment partition size\n" );
tools/mkenvimage.c line 181
Quoted 10 lines
tools/mkenvimage.c new file +10
-0+return EXIT_FAILURE ; + } + +/* Read the raw input file and transform it */ + filebuf =malloc (sizeof (*envptr) * txt_file_stat.st_size); + ret =read (txt_fd, filebuf,sizeof (*envptr) * txt_file_stat.st_size); +if (ret != txt_file_stat.st_size) { +fprintf (stderr,"Can't read the whole input file\n" ); +return EXIT_FAILURE ; + }
i think the stdin logic here is broken. when you fstat() stdin, you get back st_size == 0, and so you end up doing a read() on stdin of 0 bytes, and it writes out an empty image.
simple test:
echo foo=var | ./mkenvimage -s 0x1000 -o foo -
hexdump -C foo
<see that the env is empty>
running it through strace shows the bum read(0, "", 0) ...
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110901/5e00bdeb/attachment.pgp
PATCH 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
In reply to David WagnerThis tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environment image, ready to be flashed.
use case: flash the environment with an external tool
---
changes since v5 ~~~~~~~~~~~~~~~~
- fix reading from stdin
- remove an obsolete TODO
tools/Makefile | 5 + tools/mkenvimage.c | 270 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 275 insertions(+), 0 deletions(-) create mode 100644 tools/mkenvimage.c
tools/Makefile +5 -0
@@ -66,6 +66,7 @@ BIN_FILES-$(CONFIG_BUILD_ENVCRC) += envcrc$(SFX) BIN_FILES-$(CONFIG_CMD_NET) += gen_eth_addr$(SFX) BIN_FILES-$(CONFIG_CMD_LOADS) += img2srec$(SFX) BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX) +BIN_FILES-y += mkenvimage$(SFX) BIN_FILES-y += mkimage$(SFX) BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX) BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
@@ -89,6 +90,7 @@ OBJ_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes.o NOPED_OBJ_FILES-y += kwbimage.o NOPED_OBJ_FILES-y += imximage.o NOPED_OBJ_FILES-y += omapimage.o +NOPED_OBJ_FILES-y += mkenvimage.o NOPED_OBJ_FILES-y += mkimage.o OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o NOPED_OBJ_FILES-y += os_support.o
@@ -184,6 +186,9 @@ $(obj)xway-swap-bytes$(SFX): $(obj)xway-swap-bytes.o $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ $(HOSTSTRIP) $@ +$(obj)mkenvimage$(SFX): $(obj)crc32.o $(obj)mkenvimage.o + $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ + $(obj)mkimage$(SFX): $(obj)crc32.o \ $(obj)default_image.o \ $(obj)fit_image.o \
tools/mkenvimage.c new file +270 -0
270 lines not on this page · raw message
Signature · 1 line
1.7.0.4
PATCH 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
oops, subject is missing "v6". I just resent a correct version.
David
PATCH v6 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
In reply to David WagnerThis tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environment image, ready to be flashed.
use case: flash the environment with an external tool
---
changes since v5 ~~~~~~~~~~~~~~~~
- fix reading from stdin
- remove an obsolete TODO
tools/Makefile | 5 + tools/mkenvimage.c | 270 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 275 insertions(+), 0 deletions(-) create mode 100644 tools/mkenvimage.c
tools/Makefile +5 -0
@@ -66,6 +66,7 @@ BIN_FILES-$(CONFIG_BUILD_ENVCRC) += envcrc$(SFX) BIN_FILES-$(CONFIG_CMD_NET) += gen_eth_addr$(SFX) BIN_FILES-$(CONFIG_CMD_LOADS) += img2srec$(SFX) BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX) +BIN_FILES-y += mkenvimage$(SFX) BIN_FILES-y += mkimage$(SFX) BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX) BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
@@ -89,6 +90,7 @@ OBJ_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes.o NOPED_OBJ_FILES-y += kwbimage.o NOPED_OBJ_FILES-y += imximage.o NOPED_OBJ_FILES-y += omapimage.o +NOPED_OBJ_FILES-y += mkenvimage.o NOPED_OBJ_FILES-y += mkimage.o OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o NOPED_OBJ_FILES-y += os_support.o
@@ -184,6 +186,9 @@ $(obj)xway-swap-bytes$(SFX): $(obj)xway-swap-bytes.o $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ $(HOSTSTRIP) $@ +$(obj)mkenvimage$(SFX): $(obj)crc32.o $(obj)mkenvimage.o + $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ + $(obj)mkimage$(SFX): $(obj)crc32.o \ $(obj)default_image.o \ $(obj)fit_image.o \
tools/mkenvimage.c new file +270 -0
270 lines not on this page · raw message
Signature · 1 line
1.7.0.4
PATCH v6 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
Quoted 5 lines · On Friday, September 02, 2011 04:48:03 David Wagner wrote:
This tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environment image, ready to be flashed.
use case: flash the environment with an external tool
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110902/53553ad6/attachment.pgp
PATCH v6 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
In reply to David WagnerHello,
Le Fri, 2 Sep 2011 10:48:03 +0200,
David Wagner <[email protected]> a ?crit :
tools/mkenvimage.c line 45
Quoted 1 line
tools/mkenvimage.c new file +1
-0+"-s <envrionment partition size> -o <output> <input file>\n"
^^^^^^ typo
Thomas
Signature · 4 lines
Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH v7 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
In reply to David WagnerThis tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environment image, ready to be flashed.
use case: flash the environment with an external tool
Acked-by; Mike Frysinger <[email protected]>
---
changes since v6: ~~~~~~~~~~~~~~~~~
* fix a typo
* add Mike's Acked-by and Thomas' Tested-by
tools/Makefile | 5 + tools/mkenvimage.c | 270 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 275 insertions(+), 0 deletions(-) create mode 100644 tools/mkenvimage.c
tools/Makefile +5 -0
@@ -66,6 +66,7 @@ BIN_FILES-$(CONFIG_BUILD_ENVCRC) += envcrc$(SFX) BIN_FILES-$(CONFIG_CMD_NET) += gen_eth_addr$(SFX) BIN_FILES-$(CONFIG_CMD_LOADS) += img2srec$(SFX) BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX) +BIN_FILES-y += mkenvimage$(SFX) BIN_FILES-y += mkimage$(SFX) BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX) BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
@@ -89,6 +90,7 @@ OBJ_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes.o NOPED_OBJ_FILES-y += kwbimage.o NOPED_OBJ_FILES-y += imximage.o NOPED_OBJ_FILES-y += omapimage.o +NOPED_OBJ_FILES-y += mkenvimage.o NOPED_OBJ_FILES-y += mkimage.o OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o NOPED_OBJ_FILES-y += os_support.o
@@ -184,6 +186,9 @@ $(obj)xway-swap-bytes$(SFX): $(obj)xway-swap-bytes.o $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ $(HOSTSTRIP) $@ +$(obj)mkenvimage$(SFX): $(obj)crc32.o $(obj)mkenvimage.o + $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ + $(obj)mkimage$(SFX): $(obj)crc32.o \ $(obj)default_image.o \ $(obj)fit_image.o \
tools/mkenvimage.c new file +270 -0
270 lines not on this page · raw message
Signature · 1 line
1.7.0.4
PATCH v8 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
In reply to David WagnerThis tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environment image, ready to be flashed.
use case: flash the environment with an external tool
Acked-by; Mike Frysinger <[email protected]>
---
change since v7:
add a missing header (stdlib.h)
tools/Makefile | 5 + tools/mkenvimage.c | 271 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 276 insertions(+), 0 deletions(-) create mode 100644 tools/mkenvimage.c
tools/Makefile +5 -0
@@ -66,6 +66,7 @@ BIN_FILES-$(CONFIG_BUILD_ENVCRC) += envcrc$(SFX) BIN_FILES-$(CONFIG_CMD_NET) += gen_eth_addr$(SFX) BIN_FILES-$(CONFIG_CMD_LOADS) += img2srec$(SFX) BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX) +BIN_FILES-y += mkenvimage$(SFX) BIN_FILES-y += mkimage$(SFX) BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX) BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
@@ -89,6 +90,7 @@ OBJ_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes.o NOPED_OBJ_FILES-y += kwbimage.o NOPED_OBJ_FILES-y += imximage.o NOPED_OBJ_FILES-y += omapimage.o +NOPED_OBJ_FILES-y += mkenvimage.o NOPED_OBJ_FILES-y += mkimage.o OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o NOPED_OBJ_FILES-y += os_support.o
@@ -184,6 +186,9 @@ $(obj)xway-swap-bytes$(SFX): $(obj)xway-swap-bytes.o $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ $(HOSTSTRIP) $@ +$(obj)mkenvimage$(SFX): $(obj)crc32.o $(obj)mkenvimage.o + $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ + $(obj)mkimage$(SFX): $(obj)crc32.o \ $(obj)default_image.o \ $(obj)fit_image.o \
tools/mkenvimage.c new file +271 -0
271 lines not on this page · raw message
Signature · 1 line
1.7.0.4
PATCH v8 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
Dear David Wagner,
Quoted 10 lines · In message <1317367880-13986-1-git-send-email-david.wagner@free-electrons.com>…
This tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environment image, ready to be flashed.
use case: flash the environment with an external tool
Signed-off-by David Wagner <[email protected]>Acked-by; Mike Frysinger <[email protected]>
Tested-by Thomas Petazzoni <[email protected]>---
Quoted 8 lines
change since v7:
add a missing header (
stdlib.h)tools/Makefile | 5 + tools/mkenvimage.c | 271 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 276 insertions(+), 0 deletions(-) create mode 100644 tools/mkenvimage.c
Checkpatch says:
total: 1 errors, 0 warnings, 294 lines checked
Please clean up and resubmit. Thanks.
Best regards,
Wolfgang Denk
Signature · 9 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 don't care if you *ARE* on a bondage-and-discipline post-technical
system pawned off by the nation's largest oughta-be-illegal monopoly
who cannot escape the sins of their forefathers -- namely, using the
wrong slash for directories when the C language and its brethren use
it for something else that's very important.
-- Tom Christiansen in <[email protected]>PATCH v9 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
In reply to David WagnerThis tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environment image, ready to be flashed.
use case: flash the environment with an external tool
Acked-by; Mike Frysinger <[email protected]>
---
(My first try to send it seems to have failed ; please disregard if it actually suceeded)
change since v8 ~~~~~~~~~~~~~~~
* Make checkpatch happy by putting an instruction out of a while(). Incidently, it also fixes a possible segfault (in the event of a comment being at the end of a file that hasn't an ending newline).
tools/Makefile | 5 + tools/mkenvimage.c | 272 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 277 insertions(+), 0 deletions(-) create mode 100644 tools/mkenvimage.c
tools/Makefile +5 -0
@@ -66,6 +66,7 @@ BIN_FILES-$(CONFIG_BUILD_ENVCRC) += envcrc$(SFX) BIN_FILES-$(CONFIG_CMD_NET) += gen_eth_addr$(SFX) BIN_FILES-$(CONFIG_CMD_LOADS) += img2srec$(SFX) BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX) +BIN_FILES-y += mkenvimage$(SFX) BIN_FILES-y += mkimage$(SFX) BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX) BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
@@ -89,6 +90,7 @@ OBJ_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes.o NOPED_OBJ_FILES-y += kwbimage.o NOPED_OBJ_FILES-y += imximage.o NOPED_OBJ_FILES-y += omapimage.o +NOPED_OBJ_FILES-y += mkenvimage.o NOPED_OBJ_FILES-y += mkimage.o OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o NOPED_OBJ_FILES-y += os_support.o
@@ -184,6 +186,9 @@ $(obj)xway-swap-bytes$(SFX): $(obj)xway-swap-bytes.o $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ $(HOSTSTRIP) $@ +$(obj)mkenvimage$(SFX): $(obj)crc32.o $(obj)mkenvimage.o + $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ + $(obj)mkimage$(SFX): $(obj)crc32.o \ $(obj)default_image.o \ $(obj)fit_image.o \
tools/mkenvimage.c new file +272 -0
272 lines not on this page · raw message
Signature · 1 line
1.7.5.1
PATCH v9 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
Hi David,
Quoted 7 lines
This tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environment image, ready to be flashed.
use case: flash the environment with an external tool
Signed-off-by David Wagner <[email protected]>Acked-by; Mike Frysinger <[email protected]>
There's a semi-colon instead of a colon - this might make the tools choke.
Cheers
Detlev
Signature · 7 lines
"MS Windows User?" "Yes..." "Good. Line on the left, one cross each." -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de
PATCH v10 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
In reply to David WagnerThis tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environment image, ready to be flashed.
use case: flash the environment with an external tool
---
change since v9: ~~~~~~~~~~~~~~~~
* fix a typo in the commit message spotted by Detlev Zundel
tools/Makefile | 5 + tools/mkenvimage.c | 272 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 277 insertions(+), 0 deletions(-) create mode 100644 tools/mkenvimage.c
tools/Makefile +5 -0
@@ -66,6 +66,7 @@ BIN_FILES-$(CONFIG_BUILD_ENVCRC) += envcrc$(SFX) BIN_FILES-$(CONFIG_CMD_NET) += gen_eth_addr$(SFX) BIN_FILES-$(CONFIG_CMD_LOADS) += img2srec$(SFX) BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX) +BIN_FILES-y += mkenvimage$(SFX) BIN_FILES-y += mkimage$(SFX) BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX) BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
@@ -89,6 +90,7 @@ OBJ_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes.o NOPED_OBJ_FILES-y += kwbimage.o NOPED_OBJ_FILES-y += imximage.o NOPED_OBJ_FILES-y += omapimage.o +NOPED_OBJ_FILES-y += mkenvimage.o NOPED_OBJ_FILES-y += mkimage.o OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o NOPED_OBJ_FILES-y += os_support.o
@@ -184,6 +186,9 @@ $(obj)xway-swap-bytes$(SFX): $(obj)xway-swap-bytes.o $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ $(HOSTSTRIP) $@ +$(obj)mkenvimage$(SFX): $(obj)crc32.o $(obj)mkenvimage.o + $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ + $(obj)mkimage$(SFX): $(obj)crc32.o \ $(obj)default_image.o \ $(obj)fit_image.o \
tools/mkenvimage.c new file +272 -0
272 lines not on this page · raw message
Signature · 1 line
1.7.7
PATCH v10 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
Hello,
Le Fri, 14 Oct 2011 19:16:56 +0200,
David Wagner <[email protected]> a ?crit :
Quoted 9 lines
This tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environment image, ready to be flashed.
use case: flash the environment with an external tool
Signed-off-by David Wagner <[email protected]>Acked-by Mike Frysinger <[email protected]>Tested-by Thomas Petazzoni <[email protected]>
Unless there are further comments, would it be possible to get this merged for the upcoming U-Boot, or is the merge window already closed?
Thanks!
Thomas
Signature · 4 lines
Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH v10 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
In reply to David WagnerDear David Wagner,
Quoted 4 lines · In message <1318612616-16799-1-git-send-email-david.wagner@free-electrons.com>…
This tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environment image, ready to be flashed.
use case: flash the environment with an external tool
This patch fails to build when I try to run a plain "make tools/mkenvimage":
tools/mkenvimage.c:35:22: fatal error: compiler.h: No such file or directory
tools/mkenvimage.c:39:24: fatal error: u-boot/crc.h: No such file or directory
...
Quoted 5 lines
tools/mkenvimage.c new file +5
-0+/* Parse the cmdline */ +while ((option =getopt (argc, argv,"s:o:rbp:h" )) !=-1 ) { +switch (option) { +case 's' : + datasize =strtol (optarg,NULL ,0 );
...
tools/mkenvimage.c line 105
Quoted 1 line
tools/mkenvimage.c new file +1
-0+ padbyte =strtol (optarg,NULL ,0 );
Please add error checking for invalid input formats here.
tools/mkenvimage.c line 121
Quoted 4 lines
tools/mkenvimage.c new file +4
-0+if (datasize ==0 ) { +fprintf (stderr, +"Please specify the size of the envrionnment " +"partition.\n" );
Please don't split that string, and fix the "envrionment" typo.
tools/mkenvimage.c line 128
Quoted 3 lines
tools/mkenvimage.c new file +3
-0+ dataptr =malloc (datasize *sizeof (*dataptr)); +if (!dataptr) { +fprintf (stderr,"Can't alloc dataptr.\n" );
It might be helpful to know how many bytes you were trying to allocate.
tools/mkenvimage.c line 134
Quoted 3 lines
tools/mkenvimage.c new file +3
-0+/* +* envptr points to the beginning of the actual environment (after the +* crc and possible `redundant' bit
s/bit/byte/
tools/mkenvimage.c line 146
Quoted 5 lines
tools/mkenvimage.c new file +5
-0+/* Open the input file ... */ +if (optind >= argc) { +fprintf (stderr,"Please specify an input filename\n" ); +return EXIT_FAILURE ; + }
Please also allow to use stdin as input when no "<input file>" arg is given.
tools/mkenvimage.c line 158
Quoted 8 lines
tools/mkenvimage.c new file +8
-0+int readlen =sizeof (*envptr) *2048 ; + txt_fd =STDIN_FILENO ; + +do { + filebuf =realloc (filebuf, readlen); + readbytes =read (txt_fd, filebuf + filesize, readlen); + filesize += readbytes; + }while (readbytes == readlen);
This is pretty much inefficient. Consider a size increment of something like min(readlen, 4096).
Also, realloc() can fail - add error handling.
And read() can fail, too - add error handling.
tools/mkenvimage.c line 176
Quoted 2 lines
tools/mkenvimage.c new file +2
-0+ filesize = txt_file_stat.st_size; +/* Read the raw input file and transform it */
Why don't you just use mmap() here?
tools/mkenvimage.c line 191
Quoted 3 lines
tools/mkenvimage.c new file +3
-0+if (filesize >= envsize) { +fprintf (stderr,"The input file is larger than the " +"envrionnment partition size\n" );
Please don't split such strings. See CodingStyle:
"However, never break user-visible strings such as printk
messages, because that breaks the ability to grep for them."
Please fix globally.
tools/mkenvimage.c line 225
Quoted 9 lines
tools/mkenvimage.c new file +9
-0+ }else if (filebuf[fp] =='#' ) { +if (fp !=0 && filebuf[fp-1 ] =='\n' ) { +/* This line is a comment, let's skip it */ +while (fp < txt_file_stat.st_size && + filebuf[fp] !='\n' ) + fp++; + }else { + envptr[ep++] = filebuf[fp]; + }
printenv output does not contain any such "comments".
And - aren't you also catching embedded hashes here, like in "serial#" for example?
...
tools/mkenvimage.c line 254
Quoted 6 lines
tools/mkenvimage.c new file +6
-0+ +/* Computes the CRC and put it at the beginning of the data */ + crc =crc32 (0 , envptr, envsize); + targetendian_crc = bigendian ?cpu_to_be32 (crc) :cpu_to_le32 (crc); + +memcpy (dataptr, &targetendian_crc,sizeof (uint32_t ));
I fail to see where you set the redundant flag?
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 am more bored than you could ever possibly be. Go back to work.
PATCH v10 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
Quoted 7 lines · On Sunday 23 October 2011 16:44:38 Wolfgang Denk wrote:
Quoted 5 lines · David Wagner
This tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environment image, ready to be flashed.
use case: flash the environment with an external tool
Quoted 7 lines
This patch fails to build when I try to run a plain "make
tools/mkenvimage":
tools/mkenvimage.c:35:22: fatal error:compiler.h: No such file or directorytools/mkenvimage.c:39:24: fatal error:u-boot/crc.h: No such file or directory
i think that's expected. if you do make tools, does it build correctly ? -mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111030/3fb80ddb/attachment.pgp
PATCH v10 1/1 new tool mkenvimage: generates an env image from an arbitrary config file
In reply to David WagnerQuoted 8 lines · David Wagner
This tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environment image, ready to be flashed.
use case: flash the environment with an external tool
Signed-off-by David Wagner <[email protected]>Acked-by Mike Frysinger <[email protected]>Tested-by Thomas Petazzoni <[email protected]>
Applied to u-boot-staging, sbabic at denx.de, thanks.
Best regards,
Stefano Babic
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-0 Fax: +49-8142-66989-80 Email: office at denx.de =====================================================================