[U-Boot] [PATCH 0/4] Add option -r to env import to allow import of text files with CRLF as line endings

[U-Boot] [PATCH 0/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Alexander Holler]

Hello,

time passed by, the usage of uEnv.txt is now more widespread and some
maintainers might be more experienced. So now maybe they understand why
it might be a good idea to let Windows users create a *working* uEnv.txt
too.

So here is my second and last attempt. I've added patches for some board configs
too.

Regards,

Alexander Holler

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Alexander Holler]

When this option is enabled, CRLF is treated like LF when importing environments
from text files, which means CRs ('\r') in front of LFs ('\n') are just ignored.

Drawback of enabling this option is that (maybe exported) variables which have
a trailing CR in their content will get imported without that CR. But this
drawback is very unlikely and the big advantage of letting Windows user create
a *working* uEnv.txt too is likely more welcome.

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 common/cmd_nvedit.c | 19 +++++++++++++++----
 common/env_common.c |  6 +++---
 include/search.h    |  3 ++-
 lib/hashtable.c     | 17 ++++++++++++++++-
 4 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index e6c3395..855808c 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -950,11 +950,15 @@ sep_err:
 
 #ifdef CONFIG_CMD_IMPORTENV
 /*
- * env import [-d] [-t | -b | -c] addr [size]
+ * env import [-d] [-t [-r] | -b | -c] addr [size]
  *	-d:	delete existing environment before importing;
  *		otherwise overwrite / append to existion definitions
  *	-t:	assume text format; either "size" must be given or the
  *		text data must be '\0' terminated
+ *	-r:	handle CRLF like LF, that means exported variables with
+ *		a content which ends with \r won't get imported. Used
+ *		to import text files created with editors which are using CRLF
+ *		for line endings. Only effective in addition to -t.
  *	-b:	assume binary format ('\0' separated, "\0\0" terminated)
  *	-c:	assume checksum protected environment format
  *	addr:	memory address to read from
@@ -970,6 +974,7 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,
 	int	chk = 0;
 	int	fmt = 0;
 	int	del = 0;
+	int	crlf_is_lf = 0;
 	size_t	size;
 
 	cmd = *argv;
@@ -994,6 +999,9 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,
 					goto sep_err;
 				sep = '\n';
 				break;
+			case 'r':		/* handle CRLF like LF */
+				crlf_is_lf = 1;
+				break;
 			case 'd':
 				del = 1;
 				break;
@@ -1009,6 +1017,9 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,
 	if (!fmt)
 		printf("## Warning: defaulting to text format\n");
 
+	if (sep != '\n' && crlf_is_lf )
+		crlf_is_lf = 0;
+
 	addr = simple_strtoul(argv[0], NULL, 16);
 	ptr = map_sysmem(addr, 0);
 
@@ -1050,8 +1061,8 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,
 		ptr = (char *)ep->data;
 	}
 
-	if (himport_r(&env_htab, ptr, size, sep, del ? 0 : H_NOCLEAR, 0,
-		      NULL) == 0) {
+	if (himport_r(&env_htab, ptr, size, sep, del ? 0 : H_NOCLEAR,
+			crlf_is_lf, 0, NULL) == 0) {
 		error("Environment import failed: errno = %d\n", errno);
 		return 1;
 	}
@@ -1180,7 +1191,7 @@ static char env_help_text[] =
 #endif
 #endif
 #if defined(CONFIG_CMD_IMPORTENV)
-	"env import [-d] [-t | -b | -c] addr [size] - import environment\n"
+	"env import [-d] [-t [-r] | -b | -c] addr [size] - import environment\n"
 #endif
 	"env print [-a | name ...] - print environment\n"
 #if defined(CONFIG_CMD_RUN)
diff --git a/common/env_common.c b/common/env_common.c
index cd7b4cd..af372a6 100644
--- a/common/env_common.c
+++ b/common/env_common.c
@@ -120,7 +120,7 @@ void set_default_env(const char *s)
 	}
 
 	if (himport_r(&env_htab, (char *)default_environment,
-			sizeof(default_environment), '\0', flags,
+			sizeof(default_environment), '\0', flags, 0,
 			0, NULL) == 0)
 		error("Environment import failed: errno = %d\n", errno);
 
@@ -137,7 +137,7 @@ int set_default_vars(int nvars, char * const vars[])
 	 */
 	return himport_r(&env_htab, (const char *)default_environment,
 				sizeof(default_environment), '\0',
-				H_NOCLEAR | H_INTERACTIVE, nvars, vars);
+				H_NOCLEAR | H_INTERACTIVE, 0, nvars, vars);
 }
 
 #ifdef CONFIG_ENV_AES
@@ -214,7 +214,7 @@ int env_import(const char *buf, int check)
 		return ret;
 	}
 
-	if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0,
+	if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0, 0,
 			0, NULL)) {
 		gd->flags |= GD_FLG_ENV_READY;
 		return 1;
diff --git a/include/search.h b/include/search.h
index ae3efc4..9701efb 100644
--- a/include/search.h
+++ b/include/search.h
@@ -102,7 +102,8 @@ extern ssize_t hexport_r(struct hsearch_data *__htab,
  */
 extern int himport_r(struct hsearch_data *__htab,
 		     const char *__env, size_t __size, const char __sep,
-		     int __flag, int nvars, char * const vars[]);
+		     int __flag, int __crlf_is_lf, int nvars,
+		     char * const vars[]);
 
 /* Walk the whole table calling the callback on each element */
 extern int hwalk_r(struct hsearch_data *__htab, int (*callback)(ENTRY *));
diff --git a/lib/hashtable.c b/lib/hashtable.c
index 4356b23..18ed590 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -776,7 +776,7 @@ static int drop_var_from_set(const char *name, int nvars, char * vars[])
 
 int himport_r(struct hsearch_data *htab,
 		const char *env, size_t size, const char sep, int flag,
-		int nvars, char * const vars[])
+		int crlf_is_lf, int nvars, char * const vars[])
 {
 	char *data, *sp, *dp, *name, *value;
 	char *localvars[nvars];
@@ -841,6 +841,21 @@ int himport_r(struct hsearch_data *htab,
 		}
 	}
 
+	if(!size)
+		return 1;		/* everything OK */
+	if(crlf_is_lf) {
+		/* Remove Carriage Returns in front of Line Feeds */
+		unsigned ignored_crs = 0;
+		for(;dp < data + size && *dp; ++dp) {
+			if(*dp == '\r' &&
+			   dp < data + size - 1 && *(dp+1) == '\n')
+				++ignored_crs;
+			else
+				*(dp-ignored_crs) = *dp;
+		}
+		size -= ignored_crs;
+		dp = data;
+	}
 	/* Parse environment; allow for '\0' and 'sep' as separators */
 	do {
 		ENTRY e, *rv;
-- 
1.8.3.1

[U-Boot] [U-Boot, 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Tom Rini]

On Mon, Jul 14, 2014 at 05:49:55PM +0200, Alexander Holler wrote:

> When this option is enabled, CRLF is treated like LF when importing environments
> from text files, which means CRs ('\r') in front of LFs ('\n') are just ignored.
> 
> Drawback of enabling this option is that (maybe exported) variables which have
> a trailing CR in their content will get imported without that CR. But this
> drawback is very unlikely and the big advantage of letting Windows user create
> a *working* uEnv.txt too is likely more welcome.
> 
> Signed-off-by: Alexander Holler <holler@ahsoftware.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140722/53660fe9/attachment.pgp>

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Stephen Warren]

On 07/14/2014 09:49 AM, Alexander Holler wrote:
> When this option is enabled, CRLF is treated like LF when importing environments
> from text files, which means CRs ('\r') in front of LFs ('\n') are just ignored.
>
> Drawback of enabling this option is that (maybe exported) variables which have
> a trailing CR in their content will get imported without that CR. But this
> drawback is very unlikely and the big advantage of letting Windows user create
> a *working* uEnv.txt too is likely more welcome.

This patch doesn't seem to have been applied, yet patches 2..4 in the 
series were. This means that various boot scripts use "env import -t -r 
..." which fails due to the unknown option -r...

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Tom Rini]

On Wed, Jul 30, 2014 at 04:47:57PM -0600, Stephen Warren wrote:

> On 07/14/2014 09:49 AM, Alexander Holler wrote:
> >When this option is enabled, CRLF is treated like LF when importing environments
> >from text files, which means CRs ('\r') in front of LFs ('\n') are just ignored.
> >
> >Drawback of enabling this option is that (maybe exported) variables which have
> >a trailing CR in their content will get imported without that CR. But this
> >drawback is very unlikely and the big advantage of letting Windows user create
> >a *working* uEnv.txt too is likely more welcome.
> 
> This patch doesn't seem to have been applied, yet patches 2..4 in
> the series were. This means that various boot scripts use "env
> import -t -r ..." which fails due to the unknown option -r...

commit ecd1446fe1df00d7f7b9de286dba309d93b51870
Author: Alexander Holler <holler@ahsoftware.de>
Date:   Mon Jul 14 17:49:55 2014 +0200

Add option -r to env import to allow import of text files with CRLF as line

is what I have.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140731/86e2717a/attachment.pgp>

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Stephen Warren]

On 07/31/2014 01:51 PM, Tom Rini wrote:
> On Wed, Jul 30, 2014 at 04:47:57PM -0600, Stephen Warren wrote:
>
>> On 07/14/2014 09:49 AM, Alexander Holler wrote:
>>> When this option is enabled, CRLF is treated like LF when importing environments
>> >from text files, which means CRs ('\r') in front of LFs ('\n') are just ignored.
>>>
>>> Drawback of enabling this option is that (maybe exported) variables which have
>>> a trailing CR in their content will get imported without that CR. But this
>>> drawback is very unlikely and the big advantage of letting Windows user create
>>> a *working* uEnv.txt too is likely more welcome.
>>
>> This patch doesn't seem to have been applied, yet patches 2..4 in
>> the series were. This means that various boot scripts use "env
>> import -t -r ..." which fails due to the unknown option -r...
>
> commit ecd1446fe1df00d7f7b9de286dba309d93b51870
> Author: Alexander Holler <holler@ahsoftware.de>
> Date:   Mon Jul 14 17:49:55 2014 +0200
>
> Add option -r to env import to allow import of text files with CRLF as line
>
> is what I have.

Huh, I do see that now. I must have been looking at the content of 
common/cmd_nvedit.c from the wrong branch, which didn't include that 
patch. I could have sworn I checked git history too, but evidently not. 
It is indeed clearly there right before the patches which use it. Sorry 
for the noise.

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Alexander Holler]

Am 31.07.2014 21:57, schrieb Stephen Warren:

> Huh, I do see that now. I must have been looking at the content of
> common/cmd_nvedit.c from the wrong branch, which didn't include that
> patch. I could have sworn I checked git history too, but evidently not.
> It is indeed clearly there right before the patches which use it. Sorry
> for the noise.

As I've just remembered where I did see your name before, the config for 
the rpi (as found in 2004.04) misses the uenvcmd. That's necessary to 
execute commands when using uEnv.txt.

It's easily done with something like the following:

                                "env import -t -r $loadaddr $filesize;" \
                                "if test -n \"$uenvcmd\"; then " \
                                        "echo \"Running uenvcmd ...\";" \
                                        "run uenvcmd;" \
                                "fi;" \

Regards,

Alexander Holler

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Stephen Warren]

On 08/14/2014 02:25 AM, Alexander Holler wrote:
> Am 31.07.2014 21:57, schrieb Stephen Warren:
>
>> Huh, I do see that now. I must have been looking at the content of
>> common/cmd_nvedit.c from the wrong branch, which didn't include that
>> patch. I could have sworn I checked git history too, but evidently not.
>> It is indeed clearly there right before the patches which use it. Sorry
>> for the noise.
>
> As I've just remembered where I did see your name before, the config for
> the rpi (as found in 2004.04) misses the uenvcmd. That's necessary to
> execute commands when using uEnv.txt.
>
> It's easily done with something like the following:
>
>                                 "env import -t -r $loadaddr $filesize;" \
>                                 "if test -n \"$uenvcmd\"; then " \
>                                         "echo \"Running uenvcmd ...\";" \
>                                         "run uenvcmd;" \
>                                 "fi;" \

My intention was that uEnv.txt be used to set up environment variables, 
not to allow its use for custom scripts.

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Robert Nelson]

On Thu, Aug 14, 2014 at 10:49 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 08/14/2014 02:25 AM, Alexander Holler wrote:
>>
>> Am 31.07.2014 21:57, schrieb Stephen Warren:
>>
>>> Huh, I do see that now. I must have been looking at the content of
>>> common/cmd_nvedit.c from the wrong branch, which didn't include that
>>> patch. I could have sworn I checked git history too, but evidently not.
>>> It is indeed clearly there right before the patches which use it. Sorry
>>> for the noise.
>>
>>
>> As I've just remembered where I did see your name before, the config for
>> the rpi (as found in 2004.04) misses the uenvcmd. That's necessary to
>> execute commands when using uEnv.txt.
>>
>> It's easily done with something like the following:
>>
>>                                 "env import -t -r $loadaddr $filesize;" \
>>                                 "if test -n \"$uenvcmd\"; then " \
>>                                         "echo \"Running uenvcmd ...\";" \
>>                                         "run uenvcmd;" \
>>                                 "fi;" \
>
>
> My intention was that uEnv.txt be used to set up environment variables, not
> to allow its use for custom scripts.

The check for if uenvcmd is set then run uenvcmd syntax, should really
be pushed into the distro default stuff.  As that syntax is used by
default for a lot of different targets in u-boot.  Most users who deal
with u-boot (even if they don't want to) seem to understand it.

Regards,

-- 
Robert Nelson
http://www.rcn-ee.com/

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Tom Rini]

On Thu, Aug 14, 2014 at 01:41:16PM -0500, Robert Nelson wrote:
> On Thu, Aug 14, 2014 at 10:49 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> > On 08/14/2014 02:25 AM, Alexander Holler wrote:
> >>
> >> Am 31.07.2014 21:57, schrieb Stephen Warren:
> >>
> >>> Huh, I do see that now. I must have been looking at the content of
> >>> common/cmd_nvedit.c from the wrong branch, which didn't include that
> >>> patch. I could have sworn I checked git history too, but evidently not.
> >>> It is indeed clearly there right before the patches which use it. Sorry
> >>> for the noise.
> >>
> >>
> >> As I've just remembered where I did see your name before, the config for
> >> the rpi (as found in 2004.04) misses the uenvcmd. That's necessary to
> >> execute commands when using uEnv.txt.
> >>
> >> It's easily done with something like the following:
> >>
> >>                                 "env import -t -r $loadaddr $filesize;" \
> >>                                 "if test -n \"$uenvcmd\"; then " \
> >>                                         "echo \"Running uenvcmd ...\";" \
> >>                                         "run uenvcmd;" \
> >>                                 "fi;" \
> >
> >
> > My intention was that uEnv.txt be used to set up environment variables, not
> > to allow its use for custom scripts.
> 
> The check for if uenvcmd is set then run uenvcmd syntax, should really
> be pushed into the distro default stuff.  As that syntax is used by
> default for a lot of different targets in u-boot.  Most users who deal
> with u-boot (even if they don't want to) seem to understand it.

Right.  The intention was to provide a "just do what I want you to do"
hook to the end user.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140814/16790282/attachment.pgp>

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Stephen Warren]

On 08/14/2014 12:41 PM, Robert Nelson wrote:
> On Thu, Aug 14, 2014 at 10:49 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 08/14/2014 02:25 AM, Alexander Holler wrote:
>>>
>>> Am 31.07.2014 21:57, schrieb Stephen Warren:
>>>
>>>> Huh, I do see that now. I must have been looking at the content of
>>>> common/cmd_nvedit.c from the wrong branch, which didn't include that
>>>> patch. I could have sworn I checked git history too, but evidently not.
>>>> It is indeed clearly there right before the patches which use it. Sorry
>>>> for the noise.
>>>
>>>
>>> As I've just remembered where I did see your name before, the config for
>>> the rpi (as found in 2004.04) misses the uenvcmd. That's necessary to
>>> execute commands when using uEnv.txt.
>>>
>>> It's easily done with something like the following:
>>>
>>>                                  "env import -t -r $loadaddr $filesize;" \
>>>                                  "if test -n \"$uenvcmd\"; then " \
>>>                                          "echo \"Running uenvcmd ...\";" \
>>>                                          "run uenvcmd;" \
>>>                                  "fi;" \
>>
>>
>> My intention was that uEnv.txt be used to set up environment variables, not
>> to allow its use for custom scripts.
>
> The check for if uenvcmd is set then run uenvcmd syntax, should really
> be pushed into the distro default stuff.  As that syntax is used by
> default for a lot of different targets in u-boot.  Most users who deal
> with u-boot (even if they don't want to) seem to understand it.

I don't think this is anything to do with distro defaults.

Distro defaults are intended to provide a single common interface 
between the bootloader and Linux/... distro. As such, locating and 
loading extlinux.conf fits the bill there. The whole idea is that 
distros/OSs wouldn't have to know anything about the bootloader at all; 
command script formats, etc.

uenv.txt is the opposite; it's very U-Boot specific, and more about 
internal implementation details of U-Boot. In particular, I only see a 
use-case for uenv.txt on systems that have nowhere to store the U-Boot 
environment other than in some filesystem. That's the reason the RPi 
port loads uenv.txt, so the environment can be modified somehow. Perhaps 
there's an ENV_IS_IN_FAT that could be used instead on the Pi? For 
example, none of the Tegra boards use uEnv.txt since "saveenv" to flash 
works there.

So, perhaps I could see providing include/common_bootenv.h to make the 
definition you wrote above common between boards, but I certainly would 
not expect that opting in to distro defaults automatically added support 
for uEnv.txt.

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Tom Rini]

On Thu, Aug 14, 2014 at 01:50:31PM -0600, Stephen Warren wrote:

[snip]
> uenv.txt is the opposite; it's very U-Boot specific, and more about
> internal implementation details of U-Boot. In particular, I only see
> a use-case for uenv.txt on systems that have nowhere to store the
> U-Boot environment other than in some filesystem. That's the reason
> the RPi port loads uenv.txt, so the environment can be modified
> somehow. Perhaps there's an ENV_IS_IN_FAT that could be used instead
> on the Pi? For example, none of the Tegra boards use uEnv.txt since
> "saveenv" to flash works there.

Even with ENV_IS_IN_FAT you need to be in U-Boot to modify the
environment (fw_setenv/printenv should be adaptable easily enough I
would hope, but aren't today).  uEnv.txt is the way for a user to pop
the SD card into their PC, tweak the env as needed (or fiddle some
bits), eject the card and boot their target.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140814/b1391755/attachment.pgp>

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Alexander Holler]

Am 14.08.2014 22:53, schrieb Tom Rini:
> On Thu, Aug 14, 2014 at 01:50:31PM -0600, Stephen Warren wrote:
>
> [snip]
>> uenv.txt is the opposite; it's very U-Boot specific, and more about
>> internal implementation details of U-Boot. In particular, I only see
>> a use-case for uenv.txt on systems that have nowhere to store the
>> U-Boot environment other than in some filesystem. That's the reason
>> the RPi port loads uenv.txt, so the environment can be modified
>> somehow. Perhaps there's an ENV_IS_IN_FAT that could be used instead
>> on the Pi? For example, none of the Tegra boards use uEnv.txt since
>> "saveenv" to flash works there.
>
> Even with ENV_IS_IN_FAT you need to be in U-Boot to modify the
> environment (fw_setenv/printenv should be adaptable easily enough I
> would hope, but aren't today).  uEnv.txt is the way for a user to pop
> the SD card into their PC, tweak the env as needed (or fiddle some
> bits), eject the card and boot their target.

Yes, many "developers" today (those which do buy development boards) are 
having problems to use a serial which most of the time is needed to 
access the u-boot command line. The reasons are various, most devices 
people do use don't have a serial anymore, the voltage of the serial 
changes every few years (12, 5, 3.3 and now 1.8 Volt), sometimes a 
nullmodem (just 3 wires) is needed, ...

Whatever the reason is, sometimes it can be very hard to access the 
u-boot command line. But most are able to modifying or create a file on 
disk. ;)

Regards,

Alexander Holler

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Stephen Warren]

On 08/14/2014 02:53 PM, Tom Rini wrote:
> On Thu, Aug 14, 2014 at 01:50:31PM -0600, Stephen Warren wrote:
>
> [snip]
>> uenv.txt is the opposite; it's very U-Boot specific, and more about
>> internal implementation details of U-Boot. In particular, I only see
>> a use-case for uenv.txt on systems that have nowhere to store the
>> U-Boot environment other than in some filesystem. That's the reason
>> the RPi port loads uenv.txt, so the environment can be modified
>> somehow. Perhaps there's an ENV_IS_IN_FAT that could be used instead
>> on the Pi? For example, none of the Tegra boards use uEnv.txt since
>> "saveenv" to flash works there.
>
> Even with ENV_IS_IN_FAT you need to be in U-Boot to modify the
> environment (fw_setenv/printenv should be adaptable easily enough I
> would hope, but aren't today).  uEnv.txt is the way for a user to pop
> the SD card into their PC, tweak the env as needed (or fiddle some
> bits), eject the card and boot their target.

What, you don't link binary-editing the file to fix the CRC? :-P

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Alexander Holler]

Am 14.08.2014 23:35, schrieb Stephen Warren:
> On 08/14/2014 02:53 PM, Tom Rini wrote:
>> On Thu, Aug 14, 2014 at 01:50:31PM -0600, Stephen Warren wrote:
>>
>> [snip]
>>> uenv.txt is the opposite; it's very U-Boot specific, and more about
>>> internal implementation details of U-Boot. In particular, I only see
>>> a use-case for uenv.txt on systems that have nowhere to store the
>>> U-Boot environment other than in some filesystem. That's the reason
>>> the RPi port loads uenv.txt, so the environment can be modified
>>> somehow. Perhaps there's an ENV_IS_IN_FAT that could be used instead
>>> on the Pi? For example, none of the Tegra boards use uEnv.txt since
>>> "saveenv" to flash works there.
>>
>> Even with ENV_IS_IN_FAT you need to be in U-Boot to modify the
>> environment (fw_setenv/printenv should be adaptable easily enough I
>> would hope, but aren't today).  uEnv.txt is the way for a user to pop
>> the SD card into their PC, tweak the env as needed (or fiddle some
>> bits), eject the card and boot their target.
>
> What, you don't link binary-editing the file to fix the CRC? :-P

That reminds me that I thought about adding uEnv.sha1 (and CMD_SHA1) to 
have the same protection (or even better through sha1) as boot.cmd for 
environments where it makes sense. So in regard to "untrustworthy" 
sd-cards most boards. ;)

Regards,

Alexander Holler

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Alexander Holler]

Am 14.08.2014 17:49, schrieb Stephen Warren:
> On 08/14/2014 02:25 AM, Alexander Holler wrote:

>> As I've just remembered where I did see your name before, the config for
>> the rpi (as found in 2004.04) misses the uenvcmd. That's necessary to
>> execute commands when using uEnv.txt.
>>
>> It's easily done with something like the following:
>>
>>                                 "env import -t -r $loadaddr $filesize;" \
>>                                 "if test -n \"$uenvcmd\"; then " \
>>                                         "echo \"Running uenvcmd ...\";" \
>>                                         "run uenvcmd;" \
>>                                 "fi;" \
>
> My intention was that uEnv.txt be used to set up environment variables,
> not to allow its use for custom scripts.
>

Sure. In most cases changing the predefined available variables is 
enough. But it's a very hand option if someone wants or needs to do 
stuff which can't be done by just changing some environment variables 
(one never knows what ideas people will have).

And as it now seems to be possible to write the environment back to disk 
too, one should be make sure that uenvcmd will be cleared before writing 
the environment back to disk. Personally I prefer to not let the 
bootloader write to any (real) filesystem, but just in case someone does 
so ...

Regards,

Alexander Holler

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Stephen Warren]

On 08/14/2014 01:38 PM, Alexander Holler wrote:
> Am 14.08.2014 17:49, schrieb Stephen Warren:
>> On 08/14/2014 02:25 AM, Alexander Holler wrote:
>
>>> As I've just remembered where I did see your name before, the config for
>>> the rpi (as found in 2004.04) misses the uenvcmd. That's necessary to
>>> execute commands when using uEnv.txt.
>>>
>>> It's easily done with something like the following:
>>>
>>>                                 "env import -t -r $loadaddr
>>> $filesize;" \
>>>                                 "if test -n \"$uenvcmd\"; then " \
>>>                                         "echo \"Running uenvcmd
>>> ...\";" \
>>>                                         "run uenvcmd;" \
>>>                                 "fi;" \
>>
>> My intention was that uEnv.txt be used to set up environment variables,
>> not to allow its use for custom scripts.
>>
>
> Sure. In most cases changing the predefined available variables is
> enough. But it's a very hand option if someone wants or needs to do
> stuff which can't be done by just changing some environment variables
> (one never knows what ideas people will have).

For such presumably non-standard things, why can't the user simply edit 
$bootcmd, and pre-pend whatever they want?

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Alexander Holler]

Am 14.08.2014 21:51, schrieb Stephen Warren:
> On 08/14/2014 01:38 PM, Alexander Holler wrote:
>> Am 14.08.2014 17:49, schrieb Stephen Warren:
>>> On 08/14/2014 02:25 AM, Alexander Holler wrote:
>>
>>>> As I've just remembered where I did see your name before, the config
>>>> for
>>>> the rpi (as found in 2004.04) misses the uenvcmd. That's necessary to
>>>> execute commands when using uEnv.txt.
>>>>
>>>> It's easily done with something like the following:
>>>>
>>>>                                 "env import -t -r $loadaddr
>>>> $filesize;" \
>>>>                                 "if test -n \"$uenvcmd\"; then " \
>>>>                                         "echo \"Running uenvcmd
>>>> ...\";" \
>>>>                                         "run uenvcmd;" \
>>>>                                 "fi;" \
>>>
>>> My intention was that uEnv.txt be used to set up environment variables,
>>> not to allow its use for custom scripts.
>>>
>>
>> Sure. In most cases changing the predefined available variables is
>> enough. But it's a very hand option if someone wants or needs to do
>> stuff which can't be done by just changing some environment variables
>> (one never knows what ideas people will have).
>
> For such presumably non-standard things, why can't the user simply edit
> $bootcmd, and pre-pend whatever they want?

Depends on when the bootcmd will be constructed. Usually that is done 
after having read uEnv.txt to include variables defined in uEnv.txt in 
bootcmd. So whatever bootcmd one sets in uEnv.txt, it just will be 
overwritten.

Regards,

Alexander Holler

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Stephen Warren]

On 08/14/2014 01:59 PM, Alexander Holler wrote:
> Am 14.08.2014 21:51, schrieb Stephen Warren:
>> On 08/14/2014 01:38 PM, Alexander Holler wrote:
>>> Am 14.08.2014 17:49, schrieb Stephen Warren:
>>>> On 08/14/2014 02:25 AM, Alexander Holler wrote:
>>>
>>>>> As I've just remembered where I did see your name before, the config
>>>>> for
>>>>> the rpi (as found in 2004.04) misses the uenvcmd. That's necessary to
>>>>> execute commands when using uEnv.txt.
>>>>>
>>>>> It's easily done with something like the following:
>>>>>
>>>>>                                 "env import -t -r $loadaddr
>>>>> $filesize;" \
>>>>>                                 "if test -n \"$uenvcmd\"; then " \
>>>>>                                         "echo \"Running uenvcmd
>>>>> ...\";" \
>>>>>                                         "run uenvcmd;" \
>>>>>                                 "fi;" \
>>>>
>>>> My intention was that uEnv.txt be used to set up environment variables,
>>>> not to allow its use for custom scripts.
>>>>
>>>
>>> Sure. In most cases changing the predefined available variables is
>>> enough. But it's a very hand option if someone wants or needs to do
>>> stuff which can't be done by just changing some environment variables
>>> (one never knows what ideas people will have).
>>
>> For such presumably non-standard things, why can't the user simply edit
>> $bootcmd, and pre-pend whatever they want?
>
> Depends on when the bootcmd will be constructed. Usually that is done
> after having read uEnv.txt to include variables defined in uEnv.txt in
> bootcmd. So whatever bootcmd one sets in uEnv.txt, it just will be
> overwritten.

What would over-write bootcmd? None of the boards I've looked at 
auto-generates bootcmd. bootargs perhaps (which is a string passed to 
the kernel) but not bootargs (which is a U-Boot command sequence that 
U-Boot executes automatically at boot).

If some board does auto-generate bootcmd, I'd suggest that it not. The 
static bootcmd could execute some kind of user-(or uenv-)set variable 
and/or the auto-generation of bootcmd could happen before uenv.txt was 
pulled in, so that whatever was in uenv.txt would have ultimate "power".

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Alexander Holler]

Am 14.08.2014 22:08, schrieb Stephen Warren:
> On 08/14/2014 01:59 PM, Alexander Holler wrote:
>> Am 14.08.2014 21:51, schrieb Stephen Warren:
>>> On 08/14/2014 01:38 PM, Alexander Holler wrote:
>>>> Am 14.08.2014 17:49, schrieb Stephen Warren:
>>>>> On 08/14/2014 02:25 AM, Alexander Holler wrote:
>>>>
>>>>>> As I've just remembered where I did see your name before, the config
>>>>>> for
>>>>>> the rpi (as found in 2004.04) misses the uenvcmd. That's necessary to
>>>>>> execute commands when using uEnv.txt.
>>>>>>
>>>>>> It's easily done with something like the following:
>>>>>>
>>>>>>                                 "env import -t -r $loadaddr
>>>>>> $filesize;" \
>>>>>>                                 "if test -n \"$uenvcmd\"; then " \
>>>>>>                                         "echo \"Running uenvcmd
>>>>>> ...\";" \
>>>>>>                                         "run uenvcmd;" \
>>>>>>                                 "fi;" \
>>>>>
>>>>> My intention was that uEnv.txt be used to set up environment
>>>>> variables,
>>>>> not to allow its use for custom scripts.
>>>>>
>>>>
>>>> Sure. In most cases changing the predefined available variables is
>>>> enough. But it's a very hand option if someone wants or needs to do
>>>> stuff which can't be done by just changing some environment variables
>>>> (one never knows what ideas people will have).
>>>
>>> For such presumably non-standard things, why can't the user simply edit
>>> $bootcmd, and pre-pend whatever they want?
>>
>> Depends on when the bootcmd will be constructed. Usually that is done
>> after having read uEnv.txt to include variables defined in uEnv.txt in
>> bootcmd. So whatever bootcmd one sets in uEnv.txt, it just will be
>> overwritten.
>
> What would over-write bootcmd? None of the boards I've looked at
> auto-generates bootcmd. bootargs perhaps (which is a string passed to
> the kernel) but not bootargs (which is a U-Boot command sequence that
> U-Boot executes automatically at boot).
>
> If some board does auto-generate bootcmd, I'd suggest that it not. The
> static bootcmd could execute some kind of user-(or uenv-)set variable
> and/or the auto-generation of bootcmd could happen before uenv.txt was
> pulled in, so that whatever was in uenv.txt would have ultimate "power".

Ah, yes. Sorry, I confused bootcmd with bootargs (I don't live in u-boot 
and just fiddle once a year or such with it).

But overwriting bootcmd needs to read uEnv.txt in PREBOOT (or how it is 
named). I originally have read uEnv.txt in the bootcmd itself, so 
overwriting it didn't work. But I don't want to dive too deep into that 
discussion, as I think it's up to the board-maintainers to write the 
config however they want and seem to fit for there users. I've just 
mentioned the uenvcmd, because it was the first, I've added to my u-boot 
for the rpi (to have the same interface I use with my other boards). ;)

Regards,

Alexander Holler

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Wolfgang Denk]

Dear Alexander Holler,

In message <1405352998-7707-2-git-send-email-holler@ahsoftware.de> you wrote:
> When this option is enabled, CRLF is treated like LF when importing environments
> from text files, which means CRs ('\r') in front of LFs ('\n') are just ignored.
> 
> Drawback of enabling this option is that (maybe exported) variables which have
> a trailing CR in their content will get imported without that CR. But this
> drawback is very unlikely and the big advantage of letting Windows user create
> a *working* uEnv.txt too is likely more welcome.

Should we not, for reasons of symmetry, then also extend "env export"
by such a "-r" option?

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
When it is incorrect, it is, at least *authoritatively* incorrect.
                                    - Hitchiker's Guide To The Galaxy

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Alexander Holler]

Am 01.08.2014 14:08, schrieb Wolfgang Denk:
> Dear Alexander Holler,
>
> In message <1405352998-7707-2-git-send-email-holler@ahsoftware.de> you wrote:
>> When this option is enabled, CRLF is treated like LF when importing environments
>> from text files, which means CRs ('\r') in front of LFs ('\n') are just ignored.
>>
>> Drawback of enabling this option is that (maybe exported) variables which have
>> a trailing CR in their content will get imported without that CR. But this
>> drawback is very unlikely and the big advantage of letting Windows user create
>> a *working* uEnv.txt too is likely more welcome.
>
> Should we not, for reasons of symmetry, then also extend "env export"
> by such a "-r" option?

Sorry, but I don't follow the new features of u-boot that closely.

Is it already possible to save an exported environment as (text-)file to 
some storage? Such wasn't possible when I've implemented that -r for 
"env import" and it doesn't make much sense if an exported environment 
never reaches users.

Another problem would be to decide in which format to save the 
environment. Same magic (e.g. an env var set by "env import") would be 
necessary to decided in which format to save the environment.

Regards,

Alexander Holler

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Alexander Holler]

Am 02.08.2014 23:09, schrieb Alexander Holler:
> Am 01.08.2014 14:08, schrieb Wolfgang Denk:

>> Should we not, for reasons of symmetry, then also extend "env export"
>> by such a "-r" option?
>
> Sorry, but I don't follow the new features of u-boot that closely.
>
> Is it already possible to save an exported environment as (text-)file to
> some storage? Such wasn't possible when I've implemented that -r for
> "env import" and it doesn't make much sense if an exported environment
> never reaches users.

Just to clarify: I see uEnv.txt (which only was possible through your 
env import implementation) as a read-only configuration file for u-boot, 
mainly used to easily configure the kernel-command-line from userspace. 
Something like grub.cfg or any config for other bootloaders. The 
(simple) trick with uenvcmd to execute commands is just a handy addition.

And I don't think all the necessary stuff to save a file in all the 
possible filesystems should end up in u-boot. Modifying filesystems is 
dangerous.

So from a u-boot point of view uEnv.txt is a read-only mechanism and I'm 
happy with it as such.

I just did the patch in the subject because it ended up with extremly 
hard to diagnose problems when someone created an uEnv.txt with CRLF 
using Windows. E.g. foo=bar in such an uEnv.txt was in fact foo=bar<CR> 
which was feeded to the kernel command line as foo=bar<CR> too, and the 
Linux kernel usually treads carriage returns as a normal character. So 
it treats bar<CR> as something different than bar, leading to various 
failures. And that underlying problem is almost impossible to see 
because everything (what a user pastes, kernel output, ...) looks good.

Regards,

Alexander Holler

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Wolfgang Denk]

Dear Alexander,

In message <53DE658F.5010703@ahsoftware.de> you wrote:
>
> Just to clarify: I see uEnv.txt (which only was possible through your 
> env import implementation) as a read-only configuration file for u-boot, 

This is just one of the many possible usages.

> And I don't think all the necessary stuff to save a file in all the 
> possible filesystems should end up in u-boot. Modifying filesystems is 
> dangerous.

Thius has nothing to do with exporting an environment.  The export
operation and the writing to the file system are two separate steps.
If a file system driver contains write support or not depends on the
file system code.  For the environment it does not matter.  If we have
write support, we just use it.

> So from a u-boot point of view uEnv.txt is a read-only mechanism and I'm 
> happy with it as such.

As mentioned, this is but one usage.

I think that "env import" / "env export" should be kept symmetric.


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
Any technology distinguishable from magic is insufficiently advanced.

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Alexander Holler]

Am 03.08.2014 19:51, schrieb Wolfgang Denk:
> Dear Alexander,
>
> In message <53DE658F.5010703@ahsoftware.de> you wrote:
>>
>> Just to clarify: I see uEnv.txt (which only was possible through your
>> env import implementation) as a read-only configuration file for u-boot,
>
> This is just one of the many possible usages.
>
>> And I don't think all the necessary stuff to save a file in all the
>> possible filesystems should end up in u-boot. Modifying filesystems is
>> dangerous.
>
> Thius has nothing to do with exporting an environment.  The export
> operation and the writing to the file system are two separate steps.
> If a file system driver contains write support or not depends on the
> file system code.  For the environment it does not matter.  If we have
> write support, we just use it.
>
>> So from a u-boot point of view uEnv.txt is a read-only mechanism and I'm
>> happy with it as such.
>
> As mentioned, this is but one usage.
>
> I think that "env import" / "env export" should be kept symmetric.

Using a \r\n instead of \n when -r is used for env export should be 
something like 4 liner or such.

But it would not be really symmetric. The -r for "env import" makes "env 
import" eat both formats, which means it can be used almost always, but 
using -r with "env export" would be a decision which always would be 
wrong for many people.

Of course, adding the possibility to export the environment in a 
system-foreign format (Assuming nobody boots windows using u-boot) 
doesn't really make a harm, it just adds the danger that people will use 
-r for "env export" because it is used for "env import" too, which most 
likely would be wrong for most usage scenarios.

Anyway, I don't have any other objections agains a -r for "env export", 
maybe it could be added to the TODO-list which contains documentation 
for "env *" too. ;)

Regards,

Alexander Holler

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Alexander Holler]

Am 04.08.2014 08:47, schrieb Alexander Holler:

> But it would not be really symmetric. The -r for "env import" makes "env
> import" eat both formats, which means it can be used almost always, but
> using -r with "env export" would be a decision which always would be
> wrong for many people.
>
> Of course, adding the possibility to export the environment in a
> system-foreign format (Assuming nobody boots windows using u-boot)
> doesn't really make a harm, it just adds the danger that people will use
> -r for "env export" because it is used for "env import" too, which most
> likely would be wrong for most usage scenarios.
>
> Anyway, I don't have any other objections agains a -r for "env export",
> maybe it could be added to the TODO-list which contains documentation
> for "env *" too. ;)

My slow brain (therefor this second message) now suggests to add e.g. a 
-w to "env export", where the -w would be mutually exclusive to -t. This 
would make it clear that it doesn't mean the same as '-t -r' for env 
import and that using -w means a decision which might be wrong.

Regards,

Alexander Holler

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Måns Rullgård]

Alexander Holler <holler@ahsoftware.de> writes:

> Am 03.08.2014 19:51, schrieb Wolfgang Denk:
>> Dear Alexander,
>>
>> In message <53DE658F.5010703@ahsoftware.de> you wrote:
>>>
>>> Just to clarify: I see uEnv.txt (which only was possible through your
>>> env import implementation) as a read-only configuration file for u-boot,
>>
>> This is just one of the many possible usages.
>>
>>> And I don't think all the necessary stuff to save a file in all the
>>> possible filesystems should end up in u-boot. Modifying filesystems is
>>> dangerous.
>>
>> Thius has nothing to do with exporting an environment.  The export
>> operation and the writing to the file system are two separate steps.
>> If a file system driver contains write support or not depends on the
>> file system code.  For the environment it does not matter.  If we have
>> write support, we just use it.
>>
>>> So from a u-boot point of view uEnv.txt is a read-only mechanism and I'm
>>> happy with it as such.
>>
>> As mentioned, this is but one usage.
>>
>> I think that "env import" / "env export" should be kept symmetric.
>
> Using a \r\n instead of \n when -r is used for env export should be
> something like 4 liner or such.
>
> But it would not be really symmetric. The -r for "env import" makes
> "env import" eat both formats, which means it can be used almost
> always, but using -r with "env export" would be a decision which
> always would be wrong for many people.
>
> Of course, adding the possibility to export the environment in a
> system-foreign format (Assuming nobody boots windows using u-boot)
> doesn't really make a harm, it just adds the danger that people will
> use -r for "env export" because it is used for "env import" too, which
> most likely would be wrong for most usage scenarios.

Why not just make "env import" treat \r like any other whitespace?  It
would be a slight change from current behaviour, but the chance that
someone actually relies on a trailing \r being part of the value is
vanishingly small.

-- 
M?ns Rullg?rd
mans at mansr.com

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Alexander Holler]

Am 04.08.2014 12:00, schrieb M?ns Rullg?rd:
> Alexander Holler <holler@ahsoftware.de> writes:

> Why not just make "env import" treat \r like any other whitespace?  It
> would be a slight change from current behaviour, but the chance that
> someone actually relies on a trailing \r being part of the value is
> vanishingly small.

There was an objection about such wheh I've first posted the patch 
whichg already has had the same strong check as the current patch. But I 
think nobody still has a problem with that, and whatever was said long 
time ago was based on some misunderstandings about what the patch is 
for, and how it really works and how the new feature is used.

And currently we are only talking if and how enhancing "env export".

At least I've understood it such that nobody still has an objection 
against the new feature for "env import -t" (-r).

Regards,

Alexander Holler

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Wolfgang Denk]

Dear Alexander,

In message <53DFDC99.2020206@ahsoftware.de> you wrote:
> 
> At least I've understood it such that nobody still has an objection=20
> against the new feature for "env import -t" (-r).

I object if it would be added without maintaining symmetry with "env
export".

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
"355/113 -- Not the famous irrational number PI,  but  an  incredible
simulation!"

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Alexander Holler]

Am 06.08.2014 08:43, schrieb Wolfgang Denk:
> Dear Alexander,
>
> In message <53DFDC99.2020206@ahsoftware.de> you wrote:
>>
>> At least I've understood it such that nobody still has an objection=20
>> against the new feature for "env import -t" (-r).
>
> I object if it would be added without maintaining symmetry with "env
> export".

As explained I don't know how to add symmetry. It's impossible to export 
text concurrently in both formats.

And I will not write a patch (I don't need), if I have to assume I'm 
wasting my time because it will never reach any possible user. So 
without consensus about what such a symmetry feature for "env export" 
should be and how it will look like I will not spend the (little) time 
to write a patch, nor the much large time to discuss the patch afterwards.

Regards,

Alexander Holler

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Alexander Holler]

Am 06.08.2014 12:02, schrieb Alexander Holler:
> Am 06.08.2014 08:43, schrieb Wolfgang Denk:
>> Dear Alexander,
>>
>> In message <53DFDC99.2020206@ahsoftware.de> you wrote:
>>>
>>> At least I've understood it such that nobody still has an objection=20
>>> against the new feature for "env import -t" (-r).
>>
>> I object if it would be added without maintaining symmetry with "env
>> export".
>
> As explained I don't know how to add symmetry. It's impossible to export
> text concurrently in both formats.
>
> And I will not write a patch (I don't need), if I have to assume I'm
> wasting my time because it will never reach any possible user. So
> without consensus about what such a symmetry feature for "env export"
> should be and how it will look like I will not spend the (little) time
> to write a patch, nor the much large time to discuss the patch afterwards.

Maybe it helps to explain my motivation to write the patch in the subject:

I've run into the problem once, when I had to use a Windows box to write 
an uEnv.txt, and it happens extremly seldom that I'm using Windows. But 
then I've seen several other people running into the same problem (which 
is extremly hard to identify as <CR> is usually invisible). So I though 
I'm nice and write a patch to help other people (because I can write 
such a patch and I don't need to spend much time to do so) and everyone 
will be happy about.

So to conclude I don't need the -r for "env import" myself and it just 
ended up with around a dozen mails where I had to defend and explain the 
patch. That isn't what motivates to spend time writing "maintainer 
friendly" patches and to submit them.

Regards,

Alexander Holler

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Måns Rullgård]

Alexander Holler <holler@ahsoftware.de> writes:

> Am 06.08.2014 08:43, schrieb Wolfgang Denk:
>> Dear Alexander,
>>
>> In message <53DFDC99.2020206@ahsoftware.de> you wrote:
>>>
>>> At least I've understood it such that nobody still has an objection=20
>>> against the new feature for "env import -t" (-r).
>>
>> I object if it would be added without maintaining symmetry with "env
>> export".
>
> As explained I don't know how to add symmetry. It's impossible to
> export text concurrently in both formats.

What is the problem with ignoring \r on input and not writing it on
output?

-- 
M?ns Rullg?rd
mans at mansr.com

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Alexander Holler]

Am 06.08.2014 12:44, schrieb M?ns Rullg?rd:
> Alexander Holler <holler@ahsoftware.de> writes:
>
>> Am 06.08.2014 08:43, schrieb Wolfgang Denk:
>>> Dear Alexander,
>>>
>>> In message <53DFDC99.2020206@ahsoftware.de> you wrote:
>>>>
>>>> At least I've understood it such that nobody still has an objection=20
>>>> against the new feature for "env import -t" (-r).
>>>
>>> I object if it would be added without maintaining symmetry with "env
>>> export".
>>
>> As explained I don't know how to add symmetry. It's impossible to
>> export text concurrently in both formats.
>
> What is the problem with ignoring \r on input and not writing it on
> output?
>

None. Please read the mails before.

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Alexander Holler]

Am 06.08.2014 13:18, schrieb Alexander Holler:
> Am 06.08.2014 12:44, schrieb M?ns Rullg?rd:
>> Alexander Holler <holler@ahsoftware.de> writes:
>>
>>> Am 06.08.2014 08:43, schrieb Wolfgang Denk:
>>>> Dear Alexander,
>>>>
>>>> In message <53DFDC99.2020206@ahsoftware.de> you wrote:
>>>>>
>>>>> At least I've understood it such that nobody still has an objection=20
>>>>> against the new feature for "env import -t" (-r).
>>>>
>>>> I object if it would be added without maintaining symmetry with "env
>>>> export".
>>>
>>> As explained I don't know how to add symmetry. It's impossible to
>>> export text concurrently in both formats.
>>
>> What is the problem with ignoring \r on input and not writing it on
>> output?
>>
>
> None. Please read the mails before.

And just in case it has become difficult to follow this thread because 
the subject should have changed to something else, I think what Wolfgang 
Denk wants is an option to extent "env export" to export the environment 
to text files with CRLF.

I've made a suggestion here:

http://lists.denx.de/pipermail/u-boot/2014-August/185272.html

But as there was no response until now (I'm not impatient, I'm just 
mentioning it again because I think you've missed about what Wolfgang 
Denk started to discuss). it's just an assumption from me, I don't have 
any clue what he means with symmetry in regard to that "-r".

And without any consensus I will not spend the time to write the few 
lines of source to implement that (I don't need it). As written before, 
I don't even care much if -r for "env export -t" ends up in mainline U-Boot.

So there is no problem, Wolfgang Denk and I are just discussing if and 
how to extend "env export". That is at least how I do now understand 
this thread about the simple patch I've posted some years ago.

Regards,

Alexander Holler

[U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings

[Wolfgang Denk]

Dear Alexander,

In message <53DD53A5.3010502@ahsoftware.de> you wrote:
>
> > Should we not, for reasons of symmetry, then also extend "env export"
> > by such a "-r" option?
> 
> Sorry, but I don't follow the new features of u-boot that closely.

This is not a new feature.

> Is it already possible to save an exported environment as (text-)file to 
> some storage? Such wasn't possible when I've implemented that -r for 
> "env import" and it doesn't make much sense if an exported environment 
> never reaches users.

"env import" and "env export" have always been symmetric.  In the same
way you can (and could) import the environment from a memory reagion
(not directoly form a file) you can export it to one.  So all you need
is the capability to read a file into a memory reagion resp. to write
a file from one.  Eventually file write support was not present yet
when you looked at this, but now at least (V)FAT and ext4 include
write support (fatwrite, ext4write).

> Another problem would be to decide in which format to save the 
> environment. Same magic (e.g. an env var set by "env import") would be 
> necessary to decided in which format to save the environment.

This is a decision made by the user, ha has to know what he wants to
do resp. which format the file he is reading has or the file he is
writing shall have.

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
Often it is fatal to live too long.                          - Racine

[U-Boot] [PATCH 2/4] omap3_beagle: handle import of environments in files with CRLF as line endings

[Alexander Holler]

Use the new option -r for env import.

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 include/configs/omap3_beagle.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
index 3782049..9ba031d 100644
--- a/include/configs/omap3_beagle.h
+++ b/include/configs/omap3_beagle.h
@@ -195,7 +195,7 @@
 	"bootenv=uEnv.txt\0" \
 	"loadbootenv=fatload mmc ${mmcdev} ${loadaddr} ${bootenv}\0" \
 	"importbootenv=echo Importing environment from mmc ...; " \
-		"env import -t $loadaddr $filesize\0" \
+		"env import -t -r $loadaddr $filesize\0" \
 	"ramargs=setenv bootargs console=${console} " \
 		"${optargs} " \
 		"mpurate=${mpurate} " \
-- 
1.8.3.1

[U-Boot] [U-Boot, 2/4] omap3_beagle: handle import of environments in files with CRLF as line endings

[Tom Rini]

On Mon, Jul 14, 2014 at 05:49:56PM +0200, Alexander Holler wrote:

> Use the new option -r for env import.
> 
> Signed-off-by: Alexander Holler <holler@ahsoftware.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140722/488a86ae/attachment.pgp>

[U-Boot] [PATCH 3/4] am335x_evm: handle import of environments in files with CRLF as line endings

[Alexander Holler]

Use the new option -r for env import.

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 include/configs/am335x_evm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
index a48b386..34d27c6 100644
--- a/include/configs/am335x_evm.h
+++ b/include/configs/am335x_evm.h
@@ -117,7 +117,7 @@
 	"bootenv=uEnv.txt\0" \
 	"loadbootenv=load mmc ${mmcdev} ${loadaddr} ${bootenv}\0" \
 	"importbootenv=echo Importing environment from mmc ...; " \
-		"env import -t $loadaddr $filesize\0" \
+		"env import -t -r $loadaddr $filesize\0" \
 	"ramargs=setenv bootargs console=${console} " \
 		"${optargs} " \
 		"root=${ramroot} " \
-- 
1.8.3.1

[U-Boot] [U-Boot, 3/4] am335x_evm: handle import of environments in files with CRLF as line endings

[Tom Rini]

On Mon, Jul 14, 2014 at 05:49:57PM +0200, Alexander Holler wrote:

> Use the new option -r for env import.
> 
> Signed-off-by: Alexander Holler <holler@ahsoftware.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140722/3e780802/attachment.pgp>

[U-Boot] [PATCH 4/4] rpi_b: handle import of environments in files with CRLF as line endings

[Alexander Holler]

Use the new option -r for env import.

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 include/configs/rpi_b.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/configs/rpi_b.h b/include/configs/rpi_b.h
index ff48598..60f2489 100644
--- a/include/configs/rpi_b.h
+++ b/include/configs/rpi_b.h
@@ -98,7 +98,7 @@
 #define CONFIG_SYS_CONSOLE_IS_IN_ENV
 #define CONFIG_PREBOOT \
 	"if load mmc 0:1 ${loadaddr} /uEnv.txt; then " \
-		"env import -t ${loadaddr} ${filesize}; " \
+		"env import -t -r ${loadaddr} ${filesize}; " \
 	"fi"
 
 #define ENV_DEVICE_SETTINGS \
-- 
1.8.3.1

[U-Boot] [U-Boot, 4/4] rpi_b: handle import of environments in files with CRLF as line endings

[Tom Rini]

On Mon, Jul 14, 2014 at 05:49:58PM +0200, Alexander Holler wrote:

> Use the new option -r for env import.
> 
> Signed-off-by: Alexander Holler <holler@ahsoftware.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140722/ff52b3a5/attachment.pgp>