* [U-Boot] [PATCH] env: don't add an empty key to the env hashtable
@ 2013-04-27 21:56 Lucian Cojocar
2013-04-27 22:35 ` Wolfgang Denk
0 siblings, 1 reply; 6+ messages in thread
From: Lucian Cojocar @ 2013-04-27 21:56 UTC (permalink / raw)
To: u-boot
If the environment contains an entry like "=value" "\0" we should skip
this key/value. Otherwise, U-Boot will enter in an infinite loop.
Signed-off-by: Lucian Cojocar <cojocar@gmail.com>
---
lib/hashtable.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/lib/hashtable.c b/lib/hashtable.c
index 07ebfb2..8f5a6f8 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -870,6 +870,10 @@ int himport_r(struct hsearch_data *htab,
*sp++ = '\0'; /* terminate value */
++dp;
+ /* skip this entry if the name is empty */
+ if (*name == 0)
+ continue;
+
/* Skip variables which are not supposed to be processed */
if (!drop_var_from_set(name, nvars, localvars))
continue;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] env: don't add an empty key to the env hashtable
2013-04-27 21:56 [U-Boot] [PATCH] env: don't add an empty key to the env hashtable Lucian Cojocar
@ 2013-04-27 22:35 ` Wolfgang Denk
2013-04-27 23:08 ` Lucian Cojocar
0 siblings, 1 reply; 6+ messages in thread
From: Wolfgang Denk @ 2013-04-27 22:35 UTC (permalink / raw)
To: u-boot
Dear Lucian Cojocar,
In message <1367099787-25602-1-git-send-email-cojocar@gmail.com> you wrote:
> If the environment contains an entry like "=value" "\0" we should skip
> this key/value. Otherwise, U-Boot will enter in an infinite loop.
>
> Signed-off-by: Lucian Cojocar <cojocar@gmail.com>
> ---
> lib/hashtable.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/lib/hashtable.c b/lib/hashtable.c
> index 07ebfb2..8f5a6f8 100644
> --- a/lib/hashtable.c
> +++ b/lib/hashtable.c
> @@ -870,6 +870,10 @@ int himport_r(struct hsearch_data *htab,
> *sp++ = '\0'; /* terminate value */
> ++dp;
>
> + /* skip this entry if the name is empty */
> + if (*name == 0)
> + continue;
NAK. This would be a serious error, and silently ignoring this is
downright wrong.
If such a situation should ever happen, it must cause a fatal error.
Could you please explain which exact problem you are trying to fix?
I have to admit that I cannot think of a usage szenario that would
lead to such an error.
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
You cannot propel yourself forward by patting yourself on the back.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] env: don't add an empty key to the env hashtable
2013-04-27 22:35 ` Wolfgang Denk
@ 2013-04-27 23:08 ` Lucian Cojocar
2013-04-28 9:06 ` Wolfgang Denk
0 siblings, 1 reply; 6+ messages in thread
From: Lucian Cojocar @ 2013-04-27 23:08 UTC (permalink / raw)
To: u-boot
On 04/28/2013 12:35 AM, Wolfgang Denk wrote:
> Dear Lucian Cojocar,
>
> In message <1367099787-25602-1-git-send-email-cojocar@gmail.com> you wrote:
>> If the environment contains an entry like "=value" "\0" we should skip
>> this key/value. Otherwise, U-Boot will enter in an infinite loop.
>>
>> Signed-off-by: Lucian Cojocar <cojocar@gmail.com>
>> ---
>> lib/hashtable.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/lib/hashtable.c b/lib/hashtable.c
>> index 07ebfb2..8f5a6f8 100644
>> --- a/lib/hashtable.c
>> +++ b/lib/hashtable.c
>> @@ -870,6 +870,10 @@ int himport_r(struct hsearch_data *htab,
>> *sp++ = '\0'; /* terminate value */
>> ++dp;
>>
>> + /* skip this entry if the name is empty */
>> + if (*name == 0)
>> + continue;
>
> NAK. This would be a serious error, and silently ignoring this is
> downright wrong.
>
> If such a situation should ever happen, it must cause a fatal error.
>
I agree.
> Could you please explain which exact problem you are trying to fix?
> I have to admit that I cannot think of a usage szenario that would
> lead to such an error.
I had an error (typo) in my predefined environment. Basically I had this:
#define CONFIG_EXTRA_ENV_SETTINGS \
"key" "\0" "=value" "\0"
It would be nice if U-Boot told me that there was something wrong with
my environment rather than just hang.
Thanks,
Lucian
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] env: don't add an empty key to the env hashtable
2013-04-27 23:08 ` Lucian Cojocar
@ 2013-04-28 9:06 ` Wolfgang Denk
2013-04-28 21:31 ` [U-Boot] [PATCH v2] env: throw an error when an empty key is used Lucian Cojocar
0 siblings, 1 reply; 6+ messages in thread
From: Wolfgang Denk @ 2013-04-28 9:06 UTC (permalink / raw)
To: u-boot
Dear Lucian Cojocar,
In message <517C5A5A.2010106@gmail.com> you wrote:
>
> I had an error (typo) in my predefined environment. Basically I had this:
>
> #define CONFIG_EXTRA_ENV_SETTINGS \
> "key" "\0" "=value" "\0"
>
> It would be nice if U-Boot told me that there was something wrong with
> my environment rather than just hang.
Agreed - it should throw an error.
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
We have found all life forms in the galaxy are capable of superior
development.
-- Kirk, "The Gamesters of Triskelion", stardate 3211.7
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] env: throw an error when an empty key is used
2013-04-28 9:06 ` Wolfgang Denk
@ 2013-04-28 21:31 ` Lucian Cojocar
2013-05-10 19:58 ` [U-Boot] [U-Boot, " Tom Rini
0 siblings, 1 reply; 6+ messages in thread
From: Lucian Cojocar @ 2013-04-28 21:31 UTC (permalink / raw)
To: u-boot
If the environment contains an entry like "=value" "\0" we should throw
an error when parsing the environment. Otherwise, U-Boot will enter in
an infinite loop.
Signed-off-by: Lucian Cojocar <cojocar@gmail.com>
---
Changes for v2:
- Throw an error instead of silently skip the empty key
lib/hashtable.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/lib/hashtable.c b/lib/hashtable.c
index 07ebfb2..779580b 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -870,6 +870,12 @@ int himport_r(struct hsearch_data *htab,
*sp++ = '\0'; /* terminate value */
++dp;
+ if (*name == 0) {
+ debug("INSERT: unable to use an empty key\n");
+ __set_errno(EINVAL);
+ return 0;
+ }
+
/* Skip variables which are not supposed to be processed */
if (!drop_var_from_set(name, nvars, localvars))
continue;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-10 19:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-27 21:56 [U-Boot] [PATCH] env: don't add an empty key to the env hashtable Lucian Cojocar
2013-04-27 22:35 ` Wolfgang Denk
2013-04-27 23:08 ` Lucian Cojocar
2013-04-28 9:06 ` Wolfgang Denk
2013-04-28 21:31 ` [U-Boot] [PATCH v2] env: throw an error when an empty key is used Lucian Cojocar
2013-05-10 19:58 ` [U-Boot] [U-Boot, " Tom Rini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox