[U-Boot] [PATCH 1/2] Add option -r to env import to allow import of text files with CRLF as line endings
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.
Signed-off-by: Alexander Holler holler@ahsoftware.de --- common/cmd_nvedit.c | 17 ++++++++++++++--- common/env_common.c | 4 ++-- include/search.h | 2 +- lib/hashtable.c | 23 +++++++++++++++++++---- 4 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index e1ccdd8..f136953 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -793,11 +793,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 @@ -812,6 +816,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; @@ -836,6 +841,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; @@ -851,6 +859,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 = (char *)simple_strtoul(argv[0], NULL, 16);
if (argc == 2) { @@ -888,7 +899,7 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, addr = (char *)ep->data; }
- if (himport_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR) == 0) { + if (himport_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR, crlf_is_lf) == 0) { error("Environment import failed: errno = %d\n", errno); return 1; } @@ -974,7 +985,7 @@ U_BOOT_CMD( #if defined(CONFIG_CMD_GREPENV) "env grep string [...] - search environment\n" #endif - "env import [-d] [-t | -b | -c] addr [size] - import environment\n" + "env import [-d] [-t [-r] | | -b | -c] addr [size] - import environment\n" "env print [name ...] - print environment\n" #if defined(CONFIG_CMD_RUN) "env run var [...] - run commands in an environment variable\n" diff --git a/common/env_common.c b/common/env_common.c index c33d22d..9b50ffe 100644 --- a/common/env_common.c +++ b/common/env_common.c @@ -182,7 +182,7 @@ void set_default_env(const char *s) }
if (himport_r(&env_htab, (char *)default_environment, - sizeof(default_environment), '\0', 0) == 0) + sizeof(default_environment), '\0', 0, 0) == 0) error("Environment import failed: errno = %d\n", errno);
gd->flags |= GD_FLG_ENV_READY; @@ -207,7 +207,7 @@ int env_import(const char *buf, int check) } }
- 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)) { gd->flags |= GD_FLG_ENV_READY; return 1; } diff --git a/include/search.h b/include/search.h index ef53edb..053cf07 100644 --- a/include/search.h +++ b/include/search.h @@ -96,7 +96,7 @@ 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 __flag, int __crlf_is_lf);
/* Flags for himport_r() */ #define H_NOCLEAR 1 /* do not clear hash table before importing */ diff --git a/lib/hashtable.c b/lib/hashtable.c index abd61c8..e5fa3c3 100644 --- a/lib/hashtable.c +++ b/lib/hashtable.c @@ -623,9 +623,9 @@ ssize_t hexport_r(struct hsearch_data *htab, const char sep, * (entries separated by newline characters). * * To allow for nicely formatted text input, leading white space - * (sequences of SPACE and TAB chars) is ignored, and entries starting - * (after removal of any leading white space) with a '#' character are - * considered comments and ignored. + * (sequences of SPACE and TAB chars) is ignored, all Carriage Returns + * are ignored and entries starting (after removal of any leading white + * space) with a '#' character are considered comments and ignored. * * [NOTE: this means that a variable name cannot start with a '#' * character.] @@ -639,7 +639,7 @@ ssize_t hexport_r(struct hsearch_data *htab, const char sep, */
int himport_r(struct hsearch_data *htab, - const char *env, size_t size, const char sep, int flag) + const char *env, size_t size, const char sep, int flag, int crlf_is_lf) { char *data, *sp, *dp, *name, *value;
@@ -698,6 +698,21 @@ int himport_r(struct hsearch_data *htab, } }
+ /* Remove all Carriage Returns */ + if(!size) + return 1; /* everything OK */ + if(crlf_is_lf) { + 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;
Use the new option -r for env import.
Signed-off-by: Alexander Holler holler@ahsoftware.de --- include/configs/omap3_beagle.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h index ddeb414..d8b10c2 100644 --- a/include/configs/omap3_beagle.h +++ b/include/configs/omap3_beagle.h @@ -258,7 +258,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} " \
Dear 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 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h index ddeb414..d8b10c2 100644 --- a/include/configs/omap3_beagle.h +++ b/include/configs/omap3_beagle.h @@ -258,7 +258,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" \
Not everyone importing env on beagle use broken tools ;-)
"ramargs=setenv bootargs console=${console} " \ "${optargs} " \ "mpurate=${mpurate} " \
Best regards, Marek Vasut
On 13.05.2012 19:09, Marek Vasut wrote:
Dear Alexander Holler,
Use the new option -r for env import.
Signed-off-by: Alexander Hollerholler@ahsoftware.de
include/configs/omap3_beagle.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h index ddeb414..d8b10c2 100644 --- a/include/configs/omap3_beagle.h +++ b/include/configs/omap3_beagle.h @@ -258,7 +258,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" \Not everyone importing env on beagle use broken tools ;-)
Who said that?
Regards,
Alexander
Dear Alexander Holler,
On 13.05.2012 19:09, Marek Vasut wrote:
Dear Alexander Holler,
Use the new option -r for env import.
Signed-off-by: Alexander Hollerholler@ahsoftware.de
include/configs/omap3_beagle.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h index ddeb414..d8b10c2 100644 --- a/include/configs/omap3_beagle.h +++ b/include/configs/omap3_beagle.h @@ -258,7 +258,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" \Not everyone importing env on beagle use broken tools ;-)
Who said that?
You impose your -r option on all beagle users. That means everyone (even those with not-broken) tools will now use it (unless they replaced their env).
Regards,
Alexander
Best regards, Marek Vasut
On 13.05.2012 20:52, Marek Vasut wrote:
Dear Alexander Holler,
On 13.05.2012 19:09, Marek Vasut wrote:
Dear Alexander Holler,
Use the new option -r for env import.
Signed-off-by: Alexander Hollerholler@ahsoftware.de
include/configs/omap3_beagle.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h index ddeb414..d8b10c2 100644 --- a/include/configs/omap3_beagle.h +++ b/include/configs/omap3_beagle.h @@ -258,7 +258,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" \Not everyone importing env on beagle use broken tools ;-)
Who said that?
You impose your -r option on all beagle users. That means everyone (even those with not-broken) tools will now use it (unless they replaced their env).
Yes, and now they all will get doomed by ignored CRs before LFs.
Anyway, just ignore those patches, it was lost time from the beginning and I should have known it better.
Alexander
Dear Marek Vasut,
In message 201205131909.49488.marek.vasut@gmail.com you wrote:
"env import -t $loadaddr $filesize\0" \
"env import -t -r $loadaddr $filesize\0" \Not everyone importing env on beagle use broken tools ;-)
It's not a problem of using broken tools - the problem is of ignorant people _not_ using decade old, working tools like dos2unix if they need them.
As I mentionewd a coule of times before, I am seriously tempted to ignore this "problem".
On the other hand, we usually say we accept code if it helps some, and doesn't hurt others. As is, the increase of code size is hurting.
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
Dear Marek Vasut,
In message 201205131909.49488.marek.vasut@gmail.com you wrote:
"env import -t $loadaddr $filesize\0" \
"env import -t -r $loadaddr $filesize\0" \Not everyone importing env on beagle use broken tools ;-)
It's not a problem of using broken tools - the problem is of ignorant people _not_ using decade old, working tools like dos2unix if they need them.
tr -d '\r' won't work? :-)
As I mentionewd a coule of times before, I am seriously tempted to ignore this "problem".
Can't the wincrap people be taught to use cygwin? Or possibly some Windows (R) (C)(TM)(???) rebuilt version of tr -d '\r' ?
On the other hand, we usually say we accept code if it helps some, and doesn't hurt others. As is, the increase of code size is hurting.
Best regards,
Wolfgang Denk
Best regards, Marek Vasut
Dear Marek Vasut,
In message 201205132057.47247.marex@denx.de you wrote:
It's not a problem of using broken tools - the problem is of ignorant people _not_ using decade old, working tools like dos2unix if they need them.
tr -d '\r' won't work? :-)
Not as well - it will delete _all_ ocurrences of \r, which is more than just changing "\r\n" into "\n" (which still may be wrong in some cases).
But yes, TIMTOWTDI.
Can't the wincrap people be taught to use cygwin? Or possibly some Windows (R) (C)(TM)(???) rebuilt version of tr -d '\r' ?
Do you care?
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
Dear Marek Vasut,
In message 201205132057.47247.marex@denx.de you wrote:
It's not a problem of using broken tools - the problem is of ignorant people _not_ using decade old, working tools like dos2unix if they need them.
tr -d '\r' won't work? :-)
Not as well - it will delete _all_ ocurrences of \r, which is more than just changing "\r\n" into "\n" (which still may be wrong in some cases).
But yes, TIMTOWTDI.
What's this NLA (or 9LA) ? :)
Can't the wincrap people be taught to use cygwin? Or possibly some Windows (R) (C)(TM)(???) rebuilt version of tr -d '\r' ?
Do you care?
Well Alexander does, so I'm trying to find a fitting solution for both parties (uboot and him).
Best regards,
Wolfgang Denk
Best regards, Marek Vasut
Dear Marek Vasut,
In message 201205132145.19997.marex@denx.de you wrote:
But yes, TIMTOWTDI.
What's this NLA (or 9LA) ? :)
http://en.wikipedia.org/wiki/TIMTOWTDI
Do you care?
Well Alexander does, so I'm trying to find a fitting solution for both parties (uboot and him).
well, I suggested one: the code should be conditional, so it does not bloat code size for "all other" users. Except from that, I'd be wiling to accept it (ok, I haven't looked at the implementation yet).
Best regards,
Wolfgang Denk
On 13.05.2012 20:57, Marek Vasut wrote:
Dear Wolfgang Denk,
Dear Marek Vasut,
In message201205131909.49488.marek.vasut@gmail.com you wrote:
"env import -t $loadaddr $filesize\0" \
"env import -t -r $loadaddr $filesize\0" \Not everyone importing env on beagle use broken tools ;-)
It's not a problem of using broken tools - the problem is of ignorant people _not_ using decade old, working tools like dos2unix if they need them.
tr -d '\r' won't work? :-)
As I mentionewd a coule of times before, I am seriously tempted to ignore this "problem".
Can't the wincrap people be taught to use cygwin? Or possibly some Windows (R) (C)(TM)(???) rebuilt version of tr -d '\r' ?
Sorry, it seems you just are unable or yiu don't want to understand the problem. So here is my last message on this topic, trying to explain it for you.
env import -t is a handy solution to give users (not only devs) a possibility to modify variables (options) used to boot some OS. They don't have to deal with the u-boot command-line and, depending how it's used, env import -t is an alternative to persistent modify the env without the need for NAND.
So one just tells the user(s) that if they want to change some option they just have to create a text file on a specific place which contains a line like e.g.
foo=bar
The only problem is that such a description currently only works for Linux users. I don't know with what devices and users you are dealing, but there are some people out in the wild which don't know (much) about Linux, or even about the difference in line endings between text files creates using Windows or Linux. Some of them even just want to use their device.
And in addition, the resulting problems hare very hard to diagnose, because there will be no obvious error message when something contains a trailing CR.
It would be interesting how long you would need to diagnose such a problem without knowing about that problem, but we now will never know.
Now I needed about 10 mails to describe a problem from which I thought it's totally obvious. Sorry, but I accept that I'm totally unable to deal with this list in any, for me reasonable, time frame.
Regards,
Alexander
Dear Alexander,
In message 4FB01720.90402@ahsoftware.de you wrote:
Sorry, it seems you just are unable or yiu don't want to understand the problem. So here is my last message on this topic, trying to explain it for you.
I'm sorry you are giving up so early, just one resubmit before getting this into an acceptable state.
So one just tells the user(s) that if they want to change some option they just have to create a text file on a specific place which contains a line like e.g.
I guess this is the point where the problem starts. You should not write anywhere "they just have to create a text file", because "a text file" is not a precise enough definition of the required input format.
The only problem is that such a description currently only works for Linux users. ...
The problem is not Windows users versus Linux users here. The problem is unsufficient information.
... I don't know with what devices and users you are dealing,but there are some people out in the wild which don't know (much) about Linux, or even about the difference in line endings between text files creates using Windows or Linux. Some of them even just want to use their device.
In either case, people are usually pretty good in following pre-canned recipies for doing things.
Instead of "create a text file" one could for example document that "a UNIX style text file, i. e. with only "\n" line endings (and not the DOS-derived "\r\n" line endings) is needed". One could add some description that "on Windows systems, the command ... can be used to convert a DOS file into this format" (and provide a URL where to get this tool), "while on UNIX systems like Linux the dos2unix tool can be used".
If you then add an example suitable for copy & paste you can solve a very large percentage of the propblems you see now.
And in addition, the resulting problems hare very hard to diagnose, because there will be no obvious error message when something contains a trailing CR.
What exactly are such error modes, by the way? I would expect that trailing white space is pretty much harmless for most variable settings?
Now I needed about 10 mails to describe a problem from which I thought it's totally obvious. Sorry, but I accept that I'm totally unable to deal with this list in any, for me reasonable, time frame.
What you provide is valuable input, and it really makes sense to discuss such issues. I would like to bring such discussions to a point where either we understand why your approach makes sense, or where you agree that there are alternatives that solve the problem as well (or better) as your original proposal. However, this is difficult to acchieve if you give up early. And believe me, this is taking my time (and Marek's) as much as yours. We do this not to annoy you, but because we are interested in the best possible solution.
Best regards,
Wolfgang Denk
Am 13.05.2012 23:38, schrieb Wolfgang Denk:
Dear Alexander,
In message4FB01720.90402@ahsoftware.de you wrote:
Sorry, it seems you just are unable or yiu don't want to understand the problem. So here is my last message on this topic, trying to explain it for you.
I'm sorry you are giving up so early, just one resubmit before getting this into an acceptable state.
So one just tells the user(s) that if they want to change some option they just have to create a text file on a specific place which contains a line like e.g.
I guess this is the point where the problem starts. You should not write anywhere "they just have to create a text file", because "a text file" is not a precise enough definition of the required input format.
The only problem is that such a description currently only works for Linux users. ...
The problem is not Windows users versus Linux users here. The problem is unsufficient information.
... I don't know with what devices and users you are dealing,but there are some people out in the wild which don't know (much) about Linux, or even about the difference in line endings between text files creates using Windows or Linux. Some of them even just want to use their device.
In either case, people are usually pretty good in following pre-canned recipies for doing things.
Instead of "create a text file" one could for example document that "a UNIX style text file, i. e. with only "\n" line endings (and not the DOS-derived "\r\n" line endings) is needed". One could add some description that "on Windows systems, the command ... can be used to convert a DOS file into this format" (and provide a URL where to get this tool), "while on UNIX systems like Linux the dos2unix tool can be used".
If you then add an example suitable for copy& paste you can solve a very large percentage of the propblems you see now.
Sorry, but you are just describing one of the oldest dreams (not only) in IT.
You can write whatever you wish, users are able to not read it, to misread it, to ignore it, not to understand it or it will get missing in an one-line copy of the original multine description.
Regards,
Alexander
Am 13.05.2012 23:38, schrieb Wolfgang Denk:
Dear Alexander,
In message4FB01720.90402@ahsoftware.de you wrote:
What exactly are such error modes, by the way? I would expect that trailing white space is pretty much harmless for most variable settings?
The problem already starts in u-boot itself:
a="b\r"
I'm not sure if \r on the u-boot-cmdline does work, but you know what I mean.
$a=="b"
is false
fatload $a
fails (if the file is named 'b').
...
Regards,
Alexander
Am 14.05.2012 11:07, schrieb Alexander Holler:
Am 13.05.2012 23:38, schrieb Wolfgang Denk:
Dear Alexander,
In message4FB01720.90402@ahsoftware.de you wrote:
What exactly are such error modes, by the way? I would expect that trailing white space is pretty much harmless for most variable settings?
The problem already starts in u-boot itself:
a="b\r"
I'm not sure if \r on the u-boot-cmdline does work, but you know what I mean.
$a=="b"
is false
fatload $a
fails (if the file is named 'b').
...
And
a="1\r"
isn't a number. So anything which expects a number will fail. E.g.
fatls usb 0:${a}
and every arithmetic which uses ${a}
Regards,
Alexander
Marek Vasut marex@denx.de writes:
Dear Wolfgang Denk,
Dear Marek Vasut,
In message 201205131909.49488.marek.vasut@gmail.com you wrote:
"env import -t $loadaddr $filesize\0" \
"env import -t -r $loadaddr $filesize\0" \Not everyone importing env on beagle use broken tools ;-)
It's not a problem of using broken tools - the problem is of ignorant people _not_ using decade old, working tools like dos2unix if they need them.
tr -d '\r' won't work? :-)
As I mentionewd a coule of times before, I am seriously tempted to ignore this "problem".
Can't the wincrap people be taught to use cygwin? Or possibly some Windows (R) (C)(TM)(???) rebuilt version of tr -d '\r' ?
Most text editors on windows except notepad.exe have an option for Unix-style line breaks. People who insist on using notepad.exe deserve no sympathy.
On Sun, May 13, 2012 at 02:50:07PM +0200, Alexander Holler wrote:
Use the new option -r for env import.
Signed-off-by: Alexander Holler holler@ahsoftware.de
While I am sympathetic, the right answer is to tell users to use a text editor that can save with UNIX-style newlines. NAK.
Am 14.05.2012 18:02, schrieb Tom Rini:
On Sun, May 13, 2012 at 02:50:07PM +0200, Alexander Holler wrote:
Use the new option -r for env import.
Signed-off-by: Alexander Hollerholler@ahsoftware.de
While I am sympathetic, the right answer is to tell users to use a text editor that can save with UNIX-style newlines. NAK.
As long as you don't commit another wrong patch with me as author, I accept it and stop, as already promised, with patches.
Regards
Alexander
Dear Alexander Holler,
Am 14.05.2012 18:02, schrieb Tom Rini:
On Sun, May 13, 2012 at 02:50:07PM +0200, Alexander Holler wrote:
Use the new option -r for env import.
Signed-off-by: Alexander Hollerholler@ahsoftware.de
While I am sympathetic, the right answer is to tell users to use a text editor that can save with UNIX-style newlines. NAK.
As long as you don't commit another wrong patch with me as author
What happened?
, I accept it and stop, as already promised, with patches.
Regards
Alexander _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Best regards, Marek Vasut
On 14.05.2012 20:45, Marek Vasut wrote:
Dear Alexander Holler,
Am 14.05.2012 18:02, schrieb Tom Rini:
On Sun, May 13, 2012 at 02:50:07PM +0200, Alexander Holler wrote:
Use the new option -r for env import.
Signed-off-by: Alexander Hollerholler@ahsoftware.de
While I am sympathetic, the right answer is to tell users to use a text editor that can save with UNIX-style newlines. NAK.
As long as you don't commit another wrong patch with me as author
What happened?
Someone, which I never met in realitiy before, greeted me at the FOSDEM as the one who got famous (in some circles, don't know) for tty02 (0 as in zeroMAP, the well known processor from TI).
Regards,
Alexander
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.
Signed-off-by: Alexander Holler holler@ahsoftware.de --- common/cmd_nvedit.c | 17 ++++++++++++++--- common/env_common.c | 4 ++-- include/search.h | 2 +- lib/hashtable.c | 17 ++++++++++++++++- 4 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index e1ccdd8..f136953 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -793,11 +793,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 @@ -812,6 +816,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; @@ -836,6 +841,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; @@ -851,6 +859,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 = (char *)simple_strtoul(argv[0], NULL, 16);
if (argc == 2) { @@ -888,7 +899,7 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, addr = (char *)ep->data; }
- if (himport_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR) == 0) { + if (himport_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR, crlf_is_lf) == 0) { error("Environment import failed: errno = %d\n", errno); return 1; } @@ -974,7 +985,7 @@ U_BOOT_CMD( #if defined(CONFIG_CMD_GREPENV) "env grep string [...] - search environment\n" #endif - "env import [-d] [-t | -b | -c] addr [size] - import environment\n" + "env import [-d] [-t [-r] | | -b | -c] addr [size] - import environment\n" "env print [name ...] - print environment\n" #if defined(CONFIG_CMD_RUN) "env run var [...] - run commands in an environment variable\n" diff --git a/common/env_common.c b/common/env_common.c index c33d22d..9b50ffe 100644 --- a/common/env_common.c +++ b/common/env_common.c @@ -182,7 +182,7 @@ void set_default_env(const char *s) }
if (himport_r(&env_htab, (char *)default_environment, - sizeof(default_environment), '\0', 0) == 0) + sizeof(default_environment), '\0', 0, 0) == 0) error("Environment import failed: errno = %d\n", errno);
gd->flags |= GD_FLG_ENV_READY; @@ -207,7 +207,7 @@ int env_import(const char *buf, int check) } }
- 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)) { gd->flags |= GD_FLG_ENV_READY; return 1; } diff --git a/include/search.h b/include/search.h index ef53edb..053cf07 100644 --- a/include/search.h +++ b/include/search.h @@ -96,7 +96,7 @@ 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 __flag, int __crlf_is_lf);
/* Flags for himport_r() */ #define H_NOCLEAR 1 /* do not clear hash table before importing */ diff --git a/lib/hashtable.c b/lib/hashtable.c index abd61c8..24e73a8 100644 --- a/lib/hashtable.c +++ b/lib/hashtable.c @@ -639,7 +639,7 @@ ssize_t hexport_r(struct hsearch_data *htab, const char sep, */
int himport_r(struct hsearch_data *htab, - const char *env, size_t size, const char sep, int flag) + const char *env, size_t size, const char sep, int flag, int crlf_is_lf) { char *data, *sp, *dp, *name, *value;
@@ -698,6 +698,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;
Dear Alexander Holler,
In message 1336913407-7383-1-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.
Signed-off-by: Alexander Holler holler@ahsoftware.de
common/cmd_nvedit.c | 17 ++++++++++++++--- common/env_common.c | 4 ++-- include/search.h | 2 +- lib/hashtable.c | 23 +++++++++++++++++++---- 4 files changed, 36 insertions(+), 10 deletions(-)
Please make this optional. I don't want the increased code size for all boards when probablyonly one or two will ever activate this. And please add documentation, and include a note that the use of this option is discouraged, as the problem should be addressed outside U-Boot.
Best regards,
Wolfgang Denk
participants (6)
-
Alexander Holler -
Marek Vasut -
Marek Vasut -
Måns Rullgård -
Tom Rini -
Wolfgang Denk