Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
Hi Codrin, On Fri, Jul 24, 2015 at 8:55 AM, Codrin Ciubotariu wrote: > The code from common/env_flags.c that checks if a > string has the format of a MAC address has been moved > in net/eth.c as a separate function called > eth_validate_ethaddr_str(). > > Signed-off-by: Codrin Ciubotariu > --- > > Changes for v3: > - none, new patch; > > common/env_flags.c | 15 ++- > include/net.h | 1 + > net/eth.c | 30 ++ > 3 files changed, 33 insertions(+), 13 deletions(-) Acked-by: Joe Hershberger ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
On 07/24/2015 06:55 AM, Codrin Ciubotariu wrote:
> The code from common/env_flags.c that checks if a
> string has the format of a MAC address has been moved
> in net/eth.c as a separate function called
> eth_validate_ethaddr_str().
>
> Signed-off-by: Codrin Ciubotariu
> ---
>
> Changes for v3:
> - none, new patch;
>
> common/env_flags.c | 15 ++-
> include/net.h | 1 +
> net/eth.c | 30 ++
> 3 files changed, 33 insertions(+), 13 deletions(-)
>
> diff --git a/common/env_flags.c b/common/env_flags.c
> index 5189f5b..3e39fd1 100644
> --- a/common/env_flags.c
> +++ b/common/env_flags.c
> @@ -239,19 +239,8 @@ static int _env_flags_validate_type(const char *value,
> }
> break;
> case env_flags_vartype_macaddr:
> - cur = value;
> - for (i = 0; i < 6; i++) {
> - skip_num(1, cur, &end, 2);
> - if (cur == end)
> - return -1;
> - if (cur + 2 == end && is_hex_prefix(cur))
> - return -1;
> - if (i != 5 && *end != ':')
> - return -1;
> - if (i == 5 && *end != '\0')
> - return -1;
> - cur = end + 1;
> - }
> + if (eth_validate_ethaddr_str(value))
> + return -1;
> break;
> #endif
> case env_flags_vartype_end:
> diff --git a/include/net.h b/include/net.h
> index d17173d..c487aa7 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -218,6 +218,7 @@ void eth_try_another(int first_restart); /* Change the
> device */
> void eth_set_current(void); /* set nterface to ethcur var */
>
> int eth_get_dev_index(void); /* get the device index */
> +int eth_validate_ethaddr_str(const char *addr);
> void eth_parse_enetaddr(const char *addr, uchar *enetaddr);
> int eth_getenv_enetaddr(char *name, uchar *enetaddr);
> int eth_setenv_enetaddr(char *name, const uchar *enetaddr);
> diff --git a/net/eth.c b/net/eth.c
> index 953b731..a6fdf1b 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -7,6 +7,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -19,6 +20,35 @@
>
> DECLARE_GLOBAL_DATA_PTR;
>
> +int eth_validate_ethaddr_str(const char *addr)
> +{
> + unsigned long val;
> + int i;
> + const char *cur;
> + char *end;
> +
> + if (!addr)
> + return -1;
> +
> + cur = addr;
> + for (i = 0; i < 6; i++) {
> + val = simple_strtoul(cur, &end, 16);
> + if (cur + 1 != end && cur + 2 != end)
> + return -1;
> + if (val > 0xff)
> + return -1;
> + if (cur + 2 >= end && tolower(*(cur + 1)) == 'x')
> + return -1;
> + if (i != 5 && *end != ':')
> + return -1;
> + if (i == 5 && *end != '\0')
> + return -1;
> + cur = end + 1;
> + }
> +
> + return 0;
> +}
> +
> void eth_parse_enetaddr(const char *addr, uchar *enetaddr)
> {
> char *end;
>
Codrin,
This patch breaks most SPL targets. Please reconsider the location of
eth_validate_ethaddr_str().
York
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
Hi York,
I didn't know that SPL uses net/eth.c . Could you please suggest a place for
eth_validate_ethaddr_str()?
Thanks and best regards,
Codrin
> -Original Message-
> From: Sun York-R58495
> Sent: Saturday, August 08, 2015 3:31 AM
> To: Ciubotariu Codrin Constantin-B43658; u-boot@lists.denx.de
> Cc: joe.hershber...@ni.com; Kushwaha Prabhakar-B32579
> Subject: Re: [PATCH v3 11/16] net/eth.c: Add function to validate a MAC
> address
>
> On 07/24/2015 06:55 AM, Codrin Ciubotariu wrote:
> > The code from common/env_flags.c that checks if a string has the
> > format of a MAC address has been moved in net/eth.c as a separate
> > function called eth_validate_ethaddr_str().
> >
> > Signed-off-by: Codrin Ciubotariu
> > ---
> >
> > Changes for v3:
> > - none, new patch;
> >
> > common/env_flags.c | 15 ++-
> > include/net.h | 1 +
> > net/eth.c | 30 ++
> > 3 files changed, 33 insertions(+), 13 deletions(-)
> >
> > diff --git a/common/env_flags.c b/common/env_flags.c index
> > 5189f5b..3e39fd1 100644
> > --- a/common/env_flags.c
> > +++ b/common/env_flags.c
> > @@ -239,19 +239,8 @@ static int _env_flags_validate_type(const char *value,
> > }
> > break;
> > case env_flags_vartype_macaddr:
> > - cur = value;
> > - for (i = 0; i < 6; i++) {
> > - skip_num(1, cur, &end, 2);
> > - if (cur == end)
> > - return -1;
> > - if (cur + 2 == end && is_hex_prefix(cur))
> > - return -1;
> > - if (i != 5 && *end != ':')
> > - return -1;
> > - if (i == 5 && *end != '\0')
> > - return -1;
> > - cur = end + 1;
> > - }
> > + if (eth_validate_ethaddr_str(value))
> > + return -1;
> > break;
> > #endif
> > case env_flags_vartype_end:
> > diff --git a/include/net.h b/include/net.h index d17173d..c487aa7
> > 100644
> > --- a/include/net.h
> > +++ b/include/net.h
> > @@ -218,6 +218,7 @@ void eth_try_another(int first_restart);/*
> > Change the
> device */
> > void eth_set_current(void);/* set nterface to ethcur var */
> >
> > int eth_get_dev_index(void); /* get the device index */
> > +int eth_validate_ethaddr_str(const char *addr);
> > void eth_parse_enetaddr(const char *addr, uchar *enetaddr); int
> > eth_getenv_enetaddr(char *name, uchar *enetaddr); int
> > eth_setenv_enetaddr(char *name, const uchar *enetaddr); diff --git
> > a/net/eth.c b/net/eth.c index 953b731..a6fdf1b 100644
> > --- a/net/eth.c
> > +++ b/net/eth.c
> > @@ -7,6 +7,7 @@
> > */
> >
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -19,6 +20,35 @@
> >
> > DECLARE_GLOBAL_DATA_PTR;
> >
> > +int eth_validate_ethaddr_str(const char *addr) {
> > + unsigned long val;
> > + int i;
> > + const char *cur;
> > + char *end;
> > +
> > + if (!addr)
> > + return -1;
> > +
> > + cur = addr;
> > + for (i = 0; i < 6; i++) {
> > + val = simple_strtoul(cur, &end, 16);
> > + if (cur + 1 != end && cur + 2 != end)
> > + return -1;
> > + if (val > 0xff)
> > + return -1;
> > + if (cur + 2 >= end && tolower(*(cur + 1)) == 'x')
> > + return -1;
> > + if (i != 5 && *end != ':')
> > + return -1;
> > + if (i == 5 && *end != '\0')
> > + return -1;
> > + cur = end + 1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > void eth_parse_enetaddr(const char *addr, uchar *enetaddr) {
> > char *end;
> >
>
> Codrin,
>
> This patch breaks most SPL targets. Please reconsider the location of
> eth_validate_ethaddr_str().
>
> York
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
SPL doesn't use net/eth.c. You add a call in env_flags.c.
I think you can put it in header file and use static inline, or keep it in the
same file where it is called.
Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in
Kconfig. Joe may have some good suggestion.
York
On 08/10/2015 01:44 AM, Ciubotariu Codrin Constantin-B43658 wrote:
> Hi York,
>
> I didn't know that SPL uses net/eth.c . Could you please suggest a place for
> eth_validate_ethaddr_str()?
>
> Thanks and best regards,
> Codrin
>
>> -Original Message-
>> From: Sun York-R58495
>> Sent: Saturday, August 08, 2015 3:31 AM
>> To: Ciubotariu Codrin Constantin-B43658; u-boot@lists.denx.de
>> Cc: joe.hershber...@ni.com; Kushwaha Prabhakar-B32579
>> Subject: Re: [PATCH v3 11/16] net/eth.c: Add function to validate a MAC
>> address
>>
>> On 07/24/2015 06:55 AM, Codrin Ciubotariu wrote:
>>> The code from common/env_flags.c that checks if a string has the
>>> format of a MAC address has been moved in net/eth.c as a separate
>>> function called eth_validate_ethaddr_str().
>>>
>>> Signed-off-by: Codrin Ciubotariu
>>> ---
>>>
>>> Changes for v3:
>>> - none, new patch;
>>>
>>> common/env_flags.c | 15 ++-
>>> include/net.h | 1 +
>>> net/eth.c | 30 ++
>>> 3 files changed, 33 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/common/env_flags.c b/common/env_flags.c index
>>> 5189f5b..3e39fd1 100644
>>> --- a/common/env_flags.c
>>> +++ b/common/env_flags.c
>>> @@ -239,19 +239,8 @@ static int _env_flags_validate_type(const char *value,
>>> }
>>> break;
>>> case env_flags_vartype_macaddr:
>>> - cur = value;
>>> - for (i = 0; i < 6; i++) {
>>> - skip_num(1, cur, &end, 2);
>>> - if (cur == end)
>>> - return -1;
>>> - if (cur + 2 == end && is_hex_prefix(cur))
>>> - return -1;
>>> - if (i != 5 && *end != ':')
>>> - return -1;
>>> - if (i == 5 && *end != '\0')
>>> - return -1;
>>> - cur = end + 1;
>>> - }
>>> + if (eth_validate_ethaddr_str(value))
>>> + return -1;
>>> break;
>>> #endif
>>> case env_flags_vartype_end:
>>> diff --git a/include/net.h b/include/net.h index d17173d..c487aa7
>>> 100644
>>> --- a/include/net.h
>>> +++ b/include/net.h
>>> @@ -218,6 +218,7 @@ void eth_try_another(int first_restart);/*
>>> Change the
>> device */
>>> void eth_set_current(void);/* set nterface to ethcur var */
>>>
>>> int eth_get_dev_index(void); /* get the device index */
>>> +int eth_validate_ethaddr_str(const char *addr);
>>> void eth_parse_enetaddr(const char *addr, uchar *enetaddr); int
>>> eth_getenv_enetaddr(char *name, uchar *enetaddr); int
>>> eth_setenv_enetaddr(char *name, const uchar *enetaddr); diff --git
>>> a/net/eth.c b/net/eth.c index 953b731..a6fdf1b 100644
>>> --- a/net/eth.c
>>> +++ b/net/eth.c
>>> @@ -7,6 +7,7 @@
>>> */
>>>
>>> #include
>>> +#include
>>> #include
>>> #include
>>> #include
>>> @@ -19,6 +20,35 @@
>>>
>>> DECLARE_GLOBAL_DATA_PTR;
>>>
>>> +int eth_validate_ethaddr_str(const char *addr) {
>>> + unsigned long val;
>>> + int i;
>>> + const char *cur;
>>> + char *end;
>>> +
>>> + if (!addr)
>>> + return -1;
>>> +
>>> + cur = addr;
>>> + for (i = 0; i < 6; i++) {
>>> + val = simple_strtoul(cur, &end, 16);
>>> + if (cur + 1 != end && cur + 2 != end)
>>> + return -1;
>>> + if (val > 0xff)
>>> + return -1;
>>> + if (cur + 2 >= end && tolower(*(cur + 1)) == 'x')
>>> + return -1;
>>> + if (i != 5 && *end != ':')
>>> + return -1;
>>> + if (i == 5 && *end != '\0')
>>> + return -1;
>>> + cur = end + 1;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> void eth_parse_enetaddr(const char *addr, uchar *enetaddr) {
>>> char *end;
>>>
>>
>> Codrin,
>>
>> This patch breaks most SPL targets. Please reconsider the location of
>> eth_validate_ethaddr_str().
>>
>> York
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
Too much top-posting.
On Mon, Aug 10, 2015 at 2:41 PM, York Sun wrote:
> SPL doesn't use net/eth.c. You add a call in env_flags.c.
>
> I think you can put it in header file and use static inline, or keep it in the
> same file where it is called.
That is probably fine.
> Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in
> Kconfig. Joe may have some good suggestion.
I don't think this is the reason. The problem is that net is *not*
build for SPL, but env is.
> On 08/10/2015 01:44 AM, Ciubotariu Codrin Constantin-B43658 wrote:
>> Hi York,
>>
>> I didn't know that SPL uses net/eth.c . Could you please suggest a place for
>> eth_validate_ethaddr_str()?
>>
>> Thanks and best regards,
>> Codrin
>>
>>> -Original Message-
>>> From: Sun York-R58495
>>> Sent: Saturday, August 08, 2015 3:31 AM
>>> To: Ciubotariu Codrin Constantin-B43658; u-boot@lists.denx.de
>>> Cc: joe.hershber...@ni.com; Kushwaha Prabhakar-B32579
>>> Subject: Re: [PATCH v3 11/16] net/eth.c: Add function to validate a MAC
>>> address
>>>
>>> On 07/24/2015 06:55 AM, Codrin Ciubotariu wrote:
The code from common/env_flags.c that checks if a string has the
format of a MAC address has been moved in net/eth.c as a separate
function called eth_validate_ethaddr_str().
Signed-off-by: Codrin Ciubotariu
---
Changes for v3:
- none, new patch;
common/env_flags.c | 15 ++-
include/net.h | 1 +
net/eth.c | 30 ++
3 files changed, 33 insertions(+), 13 deletions(-)
diff --git a/common/env_flags.c b/common/env_flags.c index
5189f5b..3e39fd1 100644
--- a/common/env_flags.c
+++ b/common/env_flags.c
@@ -239,19 +239,8 @@ static int _env_flags_validate_type(const char *value,
}
break;
case env_flags_vartype_macaddr:
- cur = value;
- for (i = 0; i < 6; i++) {
- skip_num(1, cur, &end, 2);
- if (cur == end)
- return -1;
- if (cur + 2 == end && is_hex_prefix(cur))
- return -1;
- if (i != 5 && *end != ':')
- return -1;
- if (i == 5 && *end != '\0')
- return -1;
- cur = end + 1;
- }
+ if (eth_validate_ethaddr_str(value))
+ return -1;
break;
#endif
case env_flags_vartype_end:
diff --git a/include/net.h b/include/net.h index d17173d..c487aa7
100644
--- a/include/net.h
+++ b/include/net.h
@@ -218,6 +218,7 @@ void eth_try_another(int first_restart);/*
Change the
>>> device */
void eth_set_current(void);/* set nterface to ethcur var
*/
int eth_get_dev_index(void); /* get the device index */
+int eth_validate_ethaddr_str(const char *addr);
void eth_parse_enetaddr(const char *addr, uchar *enetaddr); int
eth_getenv_enetaddr(char *name, uchar *enetaddr); int
eth_setenv_enetaddr(char *name, const uchar *enetaddr); diff --git
a/net/eth.c b/net/eth.c index 953b731..a6fdf1b 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -7,6 +7,7 @@
*/
#include
+#include
#include
#include
#include
@@ -19,6 +20,35 @@
DECLARE_GLOBAL_DATA_PTR;
+int eth_validate_ethaddr_str(const char *addr) {
+ unsigned long val;
+ int i;
+ const char *cur;
+ char *end;
+
+ if (!addr)
+ return -1;
+
+ cur = addr;
+ for (i = 0; i < 6; i++) {
+ val = simple_strtoul(cur, &end, 16);
+ if (cur + 1 != end && cur + 2 != end)
+ return -1;
+ if (val > 0xff)
+ return -1;
+ if (cur + 2 >= end && tolower(*(cur + 1)) == 'x')
+ return -1;
+ if (i != 5 && *end != ':')
+ return -1;
+ if (i == 5 && *end != '\0')
+ return -1;
+ cur = end + 1;
+ }
+
+ return 0;
+}
+
void eth_parse_enetaddr(const char *addr, uchar *enetaddr) {
char *end;
>>>
>>> Codrin,
>>>
>>> This patch breaks most SPL targets. Please reconsider the location of
>>> eth_validate_ethaddr_str().
>>>
>>> York
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
On 08/10/2015 12:57 PM, Joe Hershberger wrote:
> Too much top-posting.
>
> On Mon, Aug 10, 2015 at 2:41 PM, York Sun wrote:
>> SPL doesn't use net/eth.c. You add a call in env_flags.c.
>>
>> I think you can put it in header file and use static inline, or keep it in
>> the
>> same file where it is called.
>
> That is probably fine.
>
>> Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in
>> Kconfig. Joe may have some good suggestion.
>
> I don't think this is the reason. The problem is that net is *not*
> build for SPL, but env is.
Yes, env is built. The offending lines in common/env_flags.c are gated by
"#ifdef CONFIG_CMD_NET". That's why I say it could be another way.
York
>
>> On 08/10/2015 01:44 AM, Ciubotariu Codrin Constantin-B43658 wrote:
>>> Hi York,
>>>
>>> I didn't know that SPL uses net/eth.c . Could you please suggest a place
>>> for eth_validate_ethaddr_str()?
>>>
>>> Thanks and best regards,
>>> Codrin
>>>
-Original Message-
From: Sun York-R58495
Sent: Saturday, August 08, 2015 3:31 AM
To: Ciubotariu Codrin Constantin-B43658; u-boot@lists.denx.de
Cc: joe.hershber...@ni.com; Kushwaha Prabhakar-B32579
Subject: Re: [PATCH v3 11/16] net/eth.c: Add function to validate a MAC
address
On 07/24/2015 06:55 AM, Codrin Ciubotariu wrote:
> The code from common/env_flags.c that checks if a string has the
> format of a MAC address has been moved in net/eth.c as a separate
> function called eth_validate_ethaddr_str().
>
> Signed-off-by: Codrin Ciubotariu
> ---
>
> Changes for v3:
> - none, new patch;
>
> common/env_flags.c | 15 ++-
> include/net.h | 1 +
> net/eth.c | 30 ++
> 3 files changed, 33 insertions(+), 13 deletions(-)
>
> diff --git a/common/env_flags.c b/common/env_flags.c index
> 5189f5b..3e39fd1 100644
> --- a/common/env_flags.c
> +++ b/common/env_flags.c
> @@ -239,19 +239,8 @@ static int _env_flags_validate_type(const char
> *value,
> }
> break;
> case env_flags_vartype_macaddr:
> - cur = value;
> - for (i = 0; i < 6; i++) {
> - skip_num(1, cur, &end, 2);
> - if (cur == end)
> - return -1;
> - if (cur + 2 == end && is_hex_prefix(cur))
> - return -1;
> - if (i != 5 && *end != ':')
> - return -1;
> - if (i == 5 && *end != '\0')
> - return -1;
> - cur = end + 1;
> - }
> + if (eth_validate_ethaddr_str(value))
> + return -1;
> break;
> #endif
> case env_flags_vartype_end:
> diff --git a/include/net.h b/include/net.h index d17173d..c487aa7
> 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -218,6 +218,7 @@ void eth_try_another(int first_restart);/*
> Change the
device */
> void eth_set_current(void);/* set nterface to ethcur var
> */
>
> int eth_get_dev_index(void); /* get the device index */
> +int eth_validate_ethaddr_str(const char *addr);
> void eth_parse_enetaddr(const char *addr, uchar *enetaddr); int
> eth_getenv_enetaddr(char *name, uchar *enetaddr); int
> eth_setenv_enetaddr(char *name, const uchar *enetaddr); diff --git
> a/net/eth.c b/net/eth.c index 953b731..a6fdf1b 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -7,6 +7,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -19,6 +20,35 @@
>
> DECLARE_GLOBAL_DATA_PTR;
>
> +int eth_validate_ethaddr_str(const char *addr) {
> + unsigned long val;
> + int i;
> + const char *cur;
> + char *end;
> +
> + if (!addr)
> + return -1;
> +
> + cur = addr;
> + for (i = 0; i < 6; i++) {
> + val = simple_strtoul(cur, &end, 16);
> + if (cur + 1 != end && cur + 2 != end)
> + return -1;
> + if (val > 0xff)
> + return -1;
> + if (cur + 2 >= end && tolower(*(cur + 1)) == 'x')
> + return -1;
> + if (i != 5 && *end != ':')
> + return -1;
> + if (i == 5 && *end != '\0')
> + return -1;
> + cur = end + 1;
> + }
> +
> + return 0;
> +}
> +
> void eth_parse_enetaddr(const char *addr, uchar *enetaddr) {
> char *end;
>
Codrin,
This patch breaks most SPL targets. Please reconsider the location of
eth_validat
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
Hi York,
On Mon, Aug 10, 2015 at 3:03 PM, York Sun wrote:
>
>
> On 08/10/2015 12:57 PM, Joe Hershberger wrote:
>> Too much top-posting.
>>
>> On Mon, Aug 10, 2015 at 2:41 PM, York Sun wrote:
>>> SPL doesn't use net/eth.c. You add a call in env_flags.c.
>>>
>>> I think you can put it in header file and use static inline, or keep it in
>>> the
>>> same file where it is called.
>>
>> That is probably fine.
>>
>>> Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in
>>> Kconfig. Joe may have some good suggestion.
>>
>> I don't think this is the reason. The problem is that net is *not*
>> build for SPL, but env is.
>
> Yes, env is built. The offending lines in common/env_flags.c are gated by
> "#ifdef CONFIG_CMD_NET". That's why I say it could be another way.
OK, sure... but that breaks intended behavior, I think.
>>
>>> On 08/10/2015 01:44 AM, Ciubotariu Codrin Constantin-B43658 wrote:
Hi York,
I didn't know that SPL uses net/eth.c . Could you please suggest a place
for eth_validate_ethaddr_str()?
Thanks and best regards,
Codrin
> -Original Message-
> From: Sun York-R58495
> Sent: Saturday, August 08, 2015 3:31 AM
> To: Ciubotariu Codrin Constantin-B43658; u-boot@lists.denx.de
> Cc: joe.hershber...@ni.com; Kushwaha Prabhakar-B32579
> Subject: Re: [PATCH v3 11/16] net/eth.c: Add function to validate a MAC
> address
>
> On 07/24/2015 06:55 AM, Codrin Ciubotariu wrote:
>> The code from common/env_flags.c that checks if a string has the
>> format of a MAC address has been moved in net/eth.c as a separate
>> function called eth_validate_ethaddr_str().
>>
>> Signed-off-by: Codrin Ciubotariu
>> ---
>>
>> Changes for v3:
>> - none, new patch;
>>
>> common/env_flags.c | 15 ++-
>> include/net.h | 1 +
>> net/eth.c | 30 ++
>> 3 files changed, 33 insertions(+), 13 deletions(-)
>>
>> diff --git a/common/env_flags.c b/common/env_flags.c index
>> 5189f5b..3e39fd1 100644
>> --- a/common/env_flags.c
>> +++ b/common/env_flags.c
>> @@ -239,19 +239,8 @@ static int _env_flags_validate_type(const char
>> *value,
>> }
>> break;
>> case env_flags_vartype_macaddr:
>> - cur = value;
>> - for (i = 0; i < 6; i++) {
>> - skip_num(1, cur, &end, 2);
>> - if (cur == end)
>> - return -1;
>> - if (cur + 2 == end && is_hex_prefix(cur))
>> - return -1;
>> - if (i != 5 && *end != ':')
>> - return -1;
>> - if (i == 5 && *end != '\0')
>> - return -1;
>> - cur = end + 1;
>> - }
>> + if (eth_validate_ethaddr_str(value))
>> + return -1;
>> break;
>> #endif
>> case env_flags_vartype_end:
>> diff --git a/include/net.h b/include/net.h index d17173d..c487aa7
>> 100644
>> --- a/include/net.h
>> +++ b/include/net.h
>> @@ -218,6 +218,7 @@ void eth_try_another(int first_restart);/*
>> Change the
> device */
>> void eth_set_current(void);/* set nterface to ethcur
>> var */
>>
>> int eth_get_dev_index(void); /* get the device index */
>> +int eth_validate_ethaddr_str(const char *addr);
>> void eth_parse_enetaddr(const char *addr, uchar *enetaddr); int
>> eth_getenv_enetaddr(char *name, uchar *enetaddr); int
>> eth_setenv_enetaddr(char *name, const uchar *enetaddr); diff --git
>> a/net/eth.c b/net/eth.c index 953b731..a6fdf1b 100644
>> --- a/net/eth.c
>> +++ b/net/eth.c
>> @@ -7,6 +7,7 @@
>> */
>>
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -19,6 +20,35 @@
>>
>> DECLARE_GLOBAL_DATA_PTR;
>>
>> +int eth_validate_ethaddr_str(const char *addr) {
>> + unsigned long val;
>> + int i;
>> + const char *cur;
>> + char *end;
>> +
>> + if (!addr)
>> + return -1;
>> +
>> + cur = addr;
>> + for (i = 0; i < 6; i++) {
>> + val = simple_strtoul(cur, &end, 16);
>> + if (cur + 1 != end && cur + 2 != end)
>> + return -1;
>> + if (val > 0xff)
>> + return -1;
>> + if (cur + 2 >= end && tolower(*(cur + 1)) == 'x')
>> + return -1;
>> + if (i != 5 && *end != ':')
>> + return -1;
>> + if (i == 5 && *end != '\0')
>> + return -1;
>> + cur = end + 1;
>> + }
>> +
>
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
On 08/10/2015 01:05 PM, Joe Hershberger wrote: > Hi York, > > On Mon, Aug 10, 2015 at 3:03 PM, York Sun wrote: >> >> >> On 08/10/2015 12:57 PM, Joe Hershberger wrote: >>> Too much top-posting. >>> >>> On Mon, Aug 10, 2015 at 2:41 PM, York Sun wrote: SPL doesn't use net/eth.c. You add a call in env_flags.c. I think you can put it in header file and use static inline, or keep it in the same file where it is called. >>> >>> That is probably fine. >>> Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in Kconfig. Joe may have some good suggestion. >>> >>> I don't think this is the reason. The problem is that net is *not* >>> build for SPL, but env is. >> >> Yes, env is built. The offending lines in common/env_flags.c are gated by >> "#ifdef CONFIG_CMD_NET". That's why I say it could be another way. > > OK, sure... but that breaks intended behavior, I think. > I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. So I guess the fix can be either to put the common function in header file after making it really simple to reduce dependency, or to keep the original code in env_flag.c. York ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
+Codrin Somehow I dropped Codrin in last reply. On 08/10/2015 01:45 PM, York Sun wrote: > > > On 08/10/2015 01:05 PM, Joe Hershberger wrote: >> Hi York, >> >> On Mon, Aug 10, 2015 at 3:03 PM, York Sun wrote: >>> >>> >>> On 08/10/2015 12:57 PM, Joe Hershberger wrote: Too much top-posting. On Mon, Aug 10, 2015 at 2:41 PM, York Sun wrote: > SPL doesn't use net/eth.c. You add a call in env_flags.c. > > I think you can put it in header file and use static inline, or keep it > in the > same file where it is called. That is probably fine. > Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' > in > Kconfig. Joe may have some good suggestion. I don't think this is the reason. The problem is that net is *not* build for SPL, but env is. >>> >>> Yes, env is built. The offending lines in common/env_flags.c are gated by >>> "#ifdef CONFIG_CMD_NET". That's why I say it could be another way. >> >> OK, sure... but that breaks intended behavior, I think. >> > > I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. So I guess > the fix can be either to put the common function in header file after making > it > really simple to reduce dependency, or to keep the original code in > env_flag.c. > Codrin, Can you prepare a new patch? You don't have to send the whole set. All but one have been acked by Joe. York ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
Hi York, Sure, I will make v4 this week for this change only. I will try to shrink the function and inline it in the header file. Thanks and best regards, Codrin > -Original Message- > From: Sun York-R58495 > Sent: Wednesday, August 12, 2015 10:59 PM > To: Ciubotariu Codrin Constantin-B43658 > Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com > Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a > MAC > address > > +Codrin > > Somehow I dropped Codrin in last reply. > > On 08/10/2015 01:45 PM, York Sun wrote: > > > > > > On 08/10/2015 01:05 PM, Joe Hershberger wrote: > >> Hi York, > >> > >> On Mon, Aug 10, 2015 at 3:03 PM, York Sun wrote: > >>> > >>> > >>> On 08/10/2015 12:57 PM, Joe Hershberger wrote: > >>>> Too much top-posting. > >>>> > >>>> On Mon, Aug 10, 2015 at 2:41 PM, York Sun wrote: > >>>>> SPL doesn't use net/eth.c. You add a call in env_flags.c. > >>>>> > >>>>> I think you can put it in header file and use static inline, or > >>>>> keep it in the same file where it is called. > >>>> > >>>> That is probably fine. > >>>> > >>>>> Another way is to undef CONFIG_CMD_NET for SPL part. It is default > >>>>> to 'y' in Kconfig. Joe may have some good suggestion. > >>>> > >>>> I don't think this is the reason. The problem is that net is *not* > >>>> build for SPL, but env is. > >>> > >>> Yes, env is built. The offending lines in common/env_flags.c are > >>> gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another > >>> way. > >> > >> OK, sure... but that breaks intended behavior, I think. > >> > > > > I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. So > > I guess the fix can be either to put the common function in header > > file after making it really simple to reduce dependency, or to keep the > original code in env_flag.c. > > > > Codrin, > > Can you prepare a new patch? You don't have to send the whole set. All but one > have been acked by Joe. > > York ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
Hi Joe, York, > -Original Message- > From: Sun York-R58495 > Sent: Wednesday, August 12, 2015 10:59 PM > To: Ciubotariu Codrin Constantin-B43658 > Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com > Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a > MAC > address > > +Codrin > > Somehow I dropped Codrin in last reply. > > On 08/10/2015 01:45 PM, York Sun wrote: > > > > > > On 08/10/2015 01:05 PM, Joe Hershberger wrote: > >> Hi York, > >> > >> On Mon, Aug 10, 2015 at 3:03 PM, York Sun wrote: > >>> > >>> > >>> On 08/10/2015 12:57 PM, Joe Hershberger wrote: > >>>> Too much top-posting. > >>>> > >>>> On Mon, Aug 10, 2015 at 2:41 PM, York Sun wrote: > >>>>> SPL doesn't use net/eth.c. You add a call in env_flags.c. > >>>>> > >>>>> I think you can put it in header file and use static inline, or > >>>>> keep it in the same file where it is called. > >>>> > >>>> That is probably fine. > >>>> > >>>>> Another way is to undef CONFIG_CMD_NET for SPL part. It is default > >>>>> to 'y' in Kconfig. Joe may have some good suggestion. > >>>> > >>>> I don't think this is the reason. The problem is that net is *not* > >>>> build for SPL, but env is. > >>> > >>> Yes, env is built. The offending lines in common/env_flags.c are > >>> gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another > >>> way. > >> > >> OK, sure... but that breaks intended behavior, I think. > >> > > > > I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. So > > I guess the fix can be either to put the common function in header > > file after making it really simple to reduce dependency, or to keep the > original code in env_flag.c. > > > > Codrin, > > Can you prepare a new patch? You don't have to send the whole set. All but one > have been acked by Joe. > > York I can't inline eth_validate_ethaddr_str in eth.h since it depends on simple_strtoul and tolower. Also, I can't let it in common/env_flags.c because I need to access if from drivers/net/vsc9953.c . I guess it has to be in a .c file that is also used by SPL targets. Could you please suggest such a file? SPL targets make use of CONFIG_CMD_NET? It seems strange that ETH env is built, but net/net.c or net/eth.c not. Thanks and best regards, Codrin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
On 08/13/2015 08:42 AM, Ciubotariu Codrin Constantin-B43658 wrote: > Hi Joe, York, > >> -Original Message- >> From: Sun York-R58495 >> Sent: Wednesday, August 12, 2015 10:59 PM >> To: Ciubotariu Codrin Constantin-B43658 >> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com >> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a >> MAC >> address >> >> +Codrin >> >> Somehow I dropped Codrin in last reply. >> >> On 08/10/2015 01:45 PM, York Sun wrote: >>> >>> >>> On 08/10/2015 01:05 PM, Joe Hershberger wrote: >>>> Hi York, >>>> >>>> On Mon, Aug 10, 2015 at 3:03 PM, York Sun wrote: >>>>> >>>>> >>>>> On 08/10/2015 12:57 PM, Joe Hershberger wrote: >>>>>> Too much top-posting. >>>>>> >>>>>> On Mon, Aug 10, 2015 at 2:41 PM, York Sun wrote: >>>>>>> SPL doesn't use net/eth.c. You add a call in env_flags.c. >>>>>>> >>>>>>> I think you can put it in header file and use static inline, or >>>>>>> keep it in the same file where it is called. >>>>>> >>>>>> That is probably fine. >>>>>> >>>>>>> Another way is to undef CONFIG_CMD_NET for SPL part. It is default >>>>>>> to 'y' in Kconfig. Joe may have some good suggestion. >>>>>> >>>>>> I don't think this is the reason. The problem is that net is *not* >>>>>> build for SPL, but env is. >>>>> >>>>> Yes, env is built. The offending lines in common/env_flags.c are >>>>> gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another >>>>> way. >>>> >>>> OK, sure... but that breaks intended behavior, I think. >>>> >>> >>> I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. So >>> I guess the fix can be either to put the common function in header >>> file after making it really simple to reduce dependency, or to keep the >> original code in env_flag.c. >>> >> >> Codrin, >> >> Can you prepare a new patch? You don't have to send the whole set. All but >> one >> have been acked by Joe. >> >> York > > I can't inline eth_validate_ethaddr_str in eth.h since it depends on > simple_strtoul and tolower. Also, I can't let it in common/env_flags.c > because I need to access if from drivers/net/vsc9953.c . I guess it has to be > in a .c file that is also used by SPL targets. Could you please suggest such > a file? > > SPL targets make use of CONFIG_CMD_NET? It seems strange that ETH env is > built, but net/net.c or net/eth.c not. > I was discussing with Joe about the possibility to deselect CONFIG_CMD_NET for SPL build. The issue here is Kconfig is not re-evaluated for the SPL part. If you can experiment it, you can try to gate the code in env_flags.c with CONFIG_SPL_BUILD. It sounds reasonable to me. York ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
Hi York,
> -Original Message-
> From: Sun York-R58495
> Sent: Thursday, August 13, 2015 6:55 PM
> To: Ciubotariu Codrin Constantin-B43658
> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com
> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a
> MAC
> address
>
>
>
> On 08/13/2015 08:42 AM, Ciubotariu Codrin Constantin-B43658 wrote:
> > Hi Joe, York,
> >
> >> -Original Message-
> >> From: Sun York-R58495
> >> Sent: Wednesday, August 12, 2015 10:59 PM
> >> To: Ciubotariu Codrin Constantin-B43658
> >> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com
> >> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to
> >> validate a MAC address
> >>
> >> +Codrin
> >>
> >> Somehow I dropped Codrin in last reply.
> >>
> >> On 08/10/2015 01:45 PM, York Sun wrote:
> >>>
> >>>
> >>> On 08/10/2015 01:05 PM, Joe Hershberger wrote:
> >>>> Hi York,
> >>>>
> >>>> On Mon, Aug 10, 2015 at 3:03 PM, York Sun wrote:
> >>>>>
> >>>>>
> >>>>> On 08/10/2015 12:57 PM, Joe Hershberger wrote:
> >>>>>> Too much top-posting.
> >>>>>>
> >>>>>> On Mon, Aug 10, 2015 at 2:41 PM, York Sun
> >>>>>> wrote:
> >>>>>>> SPL doesn't use net/eth.c. You add a call in env_flags.c.
> >>>>>>>
> >>>>>>> I think you can put it in header file and use static inline, or
> >>>>>>> keep it in the same file where it is called.
> >>>>>>
> >>>>>> That is probably fine.
> >>>>>>
> >>>>>>> Another way is to undef CONFIG_CMD_NET for SPL part. It is
> >>>>>>> default to 'y' in Kconfig. Joe may have some good suggestion.
> >>>>>>
> >>>>>> I don't think this is the reason. The problem is that net is
> >>>>>> *not* build for SPL, but env is.
> >>>>>
> >>>>> Yes, env is built. The offending lines in common/env_flags.c are
> >>>>> gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another
> way.
> >>>>
> >>>> OK, sure... but that breaks intended behavior, I think.
> >>>>
> >>>
> >>> I see. The CONFIG_CMD_NET is not evaluated separated for SPL build.
> >>> So I guess the fix can be either to put the common function in
> >>> header file after making it really simple to reduce dependency, or
> >>> to keep the
> >> original code in env_flag.c.
> >>>
> >>
> >> Codrin,
> >>
> >> Can you prepare a new patch? You don't have to send the whole set.
> >> All but one have been acked by Joe.
> >>
> >> York
> >
> > I can't inline eth_validate_ethaddr_str in eth.h since it depends on
> simple_strtoul and tolower. Also, I can't let it in common/env_flags.c
> because I
> need to access if from drivers/net/vsc9953.c . I guess it has to be in a .c
> file
> that is also used by SPL targets. Could you please suggest such a file?
> >
> > SPL targets make use of CONFIG_CMD_NET? It seems strange that ETH env is
> built, but net/net.c or net/eth.c not.
> >
>
> I was discussing with Joe about the possibility to deselect CONFIG_CMD_NET for
> SPL build. The issue here is Kconfig is not re-evaluated for the SPL part. If
> you can experiment it, you can try to gate the code in env_flags.c with
> CONFIG_SPL_BUILD. It sounds reasonable to me.
>
> York
Something like
#if defined(CONFIG_CMD_NET) && !defined(CONFIG_SPL_BUILD)
case env_flags_vartype_ipaddr:
cur = value;
for (i = 0; i < 4; i++) {
skip_num(0, cur, &end, 3);
if (cur == end)
return -1;
if (i != 3 && *end != '.')
return -1;
if (i == 3 && *end != '\0')
return -1;
cur = end + 1;
}
break;
case env_flags_vartype_macaddr:
if (eth_validate_ethaddr_str(value))
return -1;
break;
#endif
?
I get two warnings on this:
../common/env_flags.c: In function '_env_flags_validate_type':
../common/env_flags.c:203:2: warning: enumeration value
'env_flags_vartype_ipaddr' not handled in switch [-Wswitch]
switch (type) {
^
../common/env_flags.c:203:2: warning: enumeration value
'env_flags_vartype_macaddr' not handled in switch [-Wswitch]
Unless I guard these values in env_flags.h:
#if defined(CONFIG_CMD_NET) && !defined(CONFIG_SPL_BUILD)
env_flags_vartype_ipaddr,
env_flags_vartype_macaddr,
#endif
Best regards,
Codrin
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
On 08/14/2015 01:28 AM, Ciubotariu Codrin Constantin-B43658 wrote:
> Hi York,
>
>> -Original Message-
>> From: Sun York-R58495
>> Sent: Thursday, August 13, 2015 6:55 PM
>> To: Ciubotariu Codrin Constantin-B43658
>> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com
>> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a
>> MAC
>> address
>>
>>
>>
>> On 08/13/2015 08:42 AM, Ciubotariu Codrin Constantin-B43658 wrote:
>>> Hi Joe, York,
>>>
>>>> -Original Message-
>>>> From: Sun York-R58495
>>>> Sent: Wednesday, August 12, 2015 10:59 PM
>>>> To: Ciubotariu Codrin Constantin-B43658
>>>> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com
>>>> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to
>>>> validate a MAC address
>>>>
>>>> +Codrin
>>>>
>>>> Somehow I dropped Codrin in last reply.
>>>>
>>>> On 08/10/2015 01:45 PM, York Sun wrote:
>>>>>
>>>>>
>>>>> On 08/10/2015 01:05 PM, Joe Hershberger wrote:
>>>>>> Hi York,
>>>>>>
>>>>>> On Mon, Aug 10, 2015 at 3:03 PM, York Sun wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 08/10/2015 12:57 PM, Joe Hershberger wrote:
>>>>>>>> Too much top-posting.
>>>>>>>>
>>>>>>>> On Mon, Aug 10, 2015 at 2:41 PM, York Sun
>>>>>>>> wrote:
>>>>>>>>> SPL doesn't use net/eth.c. You add a call in env_flags.c.
>>>>>>>>>
>>>>>>>>> I think you can put it in header file and use static inline, or
>>>>>>>>> keep it in the same file where it is called.
>>>>>>>>
>>>>>>>> That is probably fine.
>>>>>>>>
>>>>>>>>> Another way is to undef CONFIG_CMD_NET for SPL part. It is
>>>>>>>>> default to 'y' in Kconfig. Joe may have some good suggestion.
>>>>>>>>
>>>>>>>> I don't think this is the reason. The problem is that net is
>>>>>>>> *not* build for SPL, but env is.
>>>>>>>
>>>>>>> Yes, env is built. The offending lines in common/env_flags.c are
>>>>>>> gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another
>> way.
>>>>>>
>>>>>> OK, sure... but that breaks intended behavior, I think.
>>>>>>
>>>>>
>>>>> I see. The CONFIG_CMD_NET is not evaluated separated for SPL build.
>>>>> So I guess the fix can be either to put the common function in
>>>>> header file after making it really simple to reduce dependency, or
>>>>> to keep the
>>>> original code in env_flag.c.
>>>>>
>>>>
>>>> Codrin,
>>>>
>>>> Can you prepare a new patch? You don't have to send the whole set.
>>>> All but one have been acked by Joe.
>>>>
>>>> York
>>>
>>> I can't inline eth_validate_ethaddr_str in eth.h since it depends on
>> simple_strtoul and tolower. Also, I can't let it in common/env_flags.c
>> because I
>> need to access if from drivers/net/vsc9953.c . I guess it has to be in a .c
>> file
>> that is also used by SPL targets. Could you please suggest such a file?
>>>
>>> SPL targets make use of CONFIG_CMD_NET? It seems strange that ETH env is
>> built, but net/net.c or net/eth.c not.
>>>
>>
>> I was discussing with Joe about the possibility to deselect CONFIG_CMD_NET
>> for
>> SPL build. The issue here is Kconfig is not re-evaluated for the SPL part. If
>> you can experiment it, you can try to gate the code in env_flags.c with
>> CONFIG_SPL_BUILD. It sounds reasonable to me.
>>
>> York
>
> Something like
>
> #if defined(CONFIG_CMD_NET) && !defined(CONFIG_SPL_BUILD)
> case env_flags_vartype_ipaddr:
> cur = value;
> for (i = 0; i < 4; i++) {
> skip_num(0, cur, &end, 3);
> if (cur == end)
> return -1;
> if (i != 3 && *end != '.')
> return -1;
> if (i == 3 && *end != '\0')
> return -1;
> cur = end + 1;
> }
> break;
> case env_flags_vartype_macaddr:
> if (eth_validate_ethaddr_str(value))
> return -1;
> break;
> #endif
>
> ?
>
> I get two warnings on this:
> ../common/env_flags.c: In function '_env_flags_validate_type':
> ../common/env_flags.c:203:2: warning: enumeration value
> 'env_flags_vartype_ipaddr' not handled in switch [-Wswitch]
> switch (type) {
> ^
> ../common/env_flags.c:203:2: warning: enumeration value
> 'env_flags_vartype_macaddr' not handled in switch [-Wswitch]
>
> Unless I guard these values in env_flags.h:
> #if defined(CONFIG_CMD_NET) && !defined(CONFIG_SPL_BUILD)
> env_flags_vartype_ipaddr,
> env_flags_vartype_macaddr,
> #endif
>
It makes sense to me to take out these two for SPL build. The whole purpose of
SPL is to load the final image. I don't see a chance users would be able to type
any command.
Joe, do you agree the CMD_NET shouldn't be used for SPL?
York
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
On Fri, Aug 14, 2015 at 12:59 PM, York Sun wrote: > > > On 08/14/2015 01:28 AM, Ciubotariu Codrin Constantin-B43658 wrote: >> Hi York, >> >>> -Original Message- >>> From: Sun York-R58495 >>> Sent: Thursday, August 13, 2015 6:55 PM >>> To: Ciubotariu Codrin Constantin-B43658 >>> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com >>> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate >>> a MAC >>> address >>> >>> >>> >>> On 08/13/2015 08:42 AM, Ciubotariu Codrin Constantin-B43658 wrote: >>>> Hi Joe, York, >>>> >>>>> -Original Message- >>>>> From: Sun York-R58495 >>>>> Sent: Wednesday, August 12, 2015 10:59 PM >>>>> To: Ciubotariu Codrin Constantin-B43658 >>>>> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com >>>>> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to >>>>> validate a MAC address >>>>> >>>>> +Codrin >>>>> >>>>> Somehow I dropped Codrin in last reply. >>>>> >>>>> On 08/10/2015 01:45 PM, York Sun wrote: >>>>>> >>>>>> >>>>>> On 08/10/2015 01:05 PM, Joe Hershberger wrote: >>>>>>> Hi York, >>>>>>> >>>>>>> On Mon, Aug 10, 2015 at 3:03 PM, York Sun wrote: >>>>>>>> >>>>>>>> >>>>>>>> On 08/10/2015 12:57 PM, Joe Hershberger wrote: >>>>>>>>> Too much top-posting. >>>>>>>>> >>>>>>>>> On Mon, Aug 10, 2015 at 2:41 PM, York Sun >>>>>>>>> wrote: >>>>>>>>>> SPL doesn't use net/eth.c. You add a call in env_flags.c. >>>>>>>>>> >>>>>>>>>> I think you can put it in header file and use static inline, or >>>>>>>>>> keep it in the same file where it is called. >>>>>>>>> >>>>>>>>> That is probably fine. >>>>>>>>> >>>>>>>>>> Another way is to undef CONFIG_CMD_NET for SPL part. It is >>>>>>>>>> default to 'y' in Kconfig. Joe may have some good suggestion. >>>>>>>>> >>>>>>>>> I don't think this is the reason. The problem is that net is >>>>>>>>> *not* build for SPL, but env is. >>>>>>>> >>>>>>>> Yes, env is built. The offending lines in common/env_flags.c are >>>>>>>> gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another >>> way. >>>>>>> >>>>>>> OK, sure... but that breaks intended behavior, I think. >>>>>>> >>>>>> >>>>>> I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. >>>>>> So I guess the fix can be either to put the common function in >>>>>> header file after making it really simple to reduce dependency, or >>>>>> to keep the >>>>> original code in env_flag.c. >>>>>> >>>>> >>>>> Codrin, >>>>> >>>>> Can you prepare a new patch? You don't have to send the whole set. >>>>> All but one have been acked by Joe. >>>>> >>>>> York >>>> >>>> I can't inline eth_validate_ethaddr_str in eth.h since it depends on >>> simple_strtoul and tolower. Also, I can't let it in common/env_flags.c >>> because I >>> need to access if from drivers/net/vsc9953.c . I guess it has to be in a .c >>> file >>> that is also used by SPL targets. Could you please suggest such a file? >>>> >>>> SPL targets make use of CONFIG_CMD_NET? It seems strange that ETH env is >>> built, but net/net.c or net/eth.c not. >>>> >>> >>> I was discussing with Joe about the possibility to deselect CONFIG_CMD_NET >>> for >>> SPL build. The issue here is Kconfig is not re-evaluated for the SPL part. >>> If >>> you can experiment it, you can try to gate the code in env_flags.c with >>> CONFIG_SPL_BUILD. It sounds reasonable to me. >>> >>> York >> >> Somethi
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
On 08/17/2015 07:37 AM, Joe Hershberger wrote: > On Fri, Aug 14, 2015 at 12:59 PM, York Sun wrote: >> >> >> On 08/14/2015 01:28 AM, Ciubotariu Codrin Constantin-B43658 wrote: >>> Hi York, >>> >>>> -Original Message- >>>> From: Sun York-R58495 >>>> Sent: Thursday, August 13, 2015 6:55 PM >>>> To: Ciubotariu Codrin Constantin-B43658 >>>> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com >>>> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate >>>> a MAC >>>> address >>>> >>>> >>>> >>>> On 08/13/2015 08:42 AM, Ciubotariu Codrin Constantin-B43658 wrote: >>>>> Hi Joe, York, >>>>> >>>>>> -Original Message----- >>>>>> From: Sun York-R58495 >>>>>> Sent: Wednesday, August 12, 2015 10:59 PM >>>>>> To: Ciubotariu Codrin Constantin-B43658 >>>>>> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com >>>>>> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to >>>>>> validate a MAC address >>>>>> >>>>>> +Codrin >>>>>> >>>>>> Somehow I dropped Codrin in last reply. >>>>>> >>>>>> On 08/10/2015 01:45 PM, York Sun wrote: >>>>>>> >>>>>>> >>>>>>> On 08/10/2015 01:05 PM, Joe Hershberger wrote: >>>>>>>> Hi York, >>>>>>>> >>>>>>>> On Mon, Aug 10, 2015 at 3:03 PM, York Sun >>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> On 08/10/2015 12:57 PM, Joe Hershberger wrote: >>>>>>>>>> Too much top-posting. >>>>>>>>>> >>>>>>>>>> On Mon, Aug 10, 2015 at 2:41 PM, York Sun >>>>>>>>>> wrote: >>>>>>>>>>> SPL doesn't use net/eth.c. You add a call in env_flags.c. >>>>>>>>>>> >>>>>>>>>>> I think you can put it in header file and use static inline, or >>>>>>>>>>> keep it in the same file where it is called. >>>>>>>>>> >>>>>>>>>> That is probably fine. >>>>>>>>>> >>>>>>>>>>> Another way is to undef CONFIG_CMD_NET for SPL part. It is >>>>>>>>>>> default to 'y' in Kconfig. Joe may have some good suggestion. >>>>>>>>>> >>>>>>>>>> I don't think this is the reason. The problem is that net is >>>>>>>>>> *not* build for SPL, but env is. >>>>>>>>> >>>>>>>>> Yes, env is built. The offending lines in common/env_flags.c are >>>>>>>>> gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another >>>> way. >>>>>>>> >>>>>>>> OK, sure... but that breaks intended behavior, I think. >>>>>>>> >>>>>>> >>>>>>> I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. >>>>>>> So I guess the fix can be either to put the common function in >>>>>>> header file after making it really simple to reduce dependency, or >>>>>>> to keep the >>>>>> original code in env_flag.c. >>>>>>> >>>>>> >>>>>> Codrin, >>>>>> >>>>>> Can you prepare a new patch? You don't have to send the whole set. >>>>>> All but one have been acked by Joe. >>>>>> >>>>>> York >>>>> >>>>> I can't inline eth_validate_ethaddr_str in eth.h since it depends on >>>> simple_strtoul and tolower. Also, I can't let it in common/env_flags.c >>>> because I >>>> need to access if from drivers/net/vsc9953.c . I guess it has to be in a >>>> .c file >>>> that is also used by SPL targets. Could you please suggest such a file? >>>>> >>>>> SPL targets make use of CONFIG_CMD_NET? It seems strange that ETH env is >>>> built, but net/net.c or net/eth.c not
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
> -Original Message- > From: Joe Hershberger [mailto:joe.hershber...@gmail.com] > Sent: Monday, August 17, 2015 5:38 PM > To: Sun York-R58495 > Cc: Ciubotariu Codrin Constantin-B43658; u-boot@lists.denx.de; > joe.hershber...@ni.com > Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a > MAC > address > > On Fri, Aug 14, 2015 at 12:59 PM, York Sun wrote: > > > > > > On 08/14/2015 01:28 AM, Ciubotariu Codrin Constantin-B43658 wrote: > >> Hi York, > >> > >>> -Original Message- > >>> From: Sun York-R58495 > >>> Sent: Thursday, August 13, 2015 6:55 PM > >>> To: Ciubotariu Codrin Constantin-B43658 > >>> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com > >>> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to > >>> validate a MAC address > >>> > >>> > >>> > >>> On 08/13/2015 08:42 AM, Ciubotariu Codrin Constantin-B43658 wrote: > >>>> Hi Joe, York, > >>>> > >>>>> -----Original Message----- > >>>>> From: Sun York-R58495 > >>>>> Sent: Wednesday, August 12, 2015 10:59 PM > >>>>> To: Ciubotariu Codrin Constantin-B43658 > >>>>> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com > >>>>> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to > >>>>> validate a MAC address > >>>>> > >>>>> +Codrin > >>>>> > >>>>> Somehow I dropped Codrin in last reply. > >>>>> > >>>>> On 08/10/2015 01:45 PM, York Sun wrote: > >>>>>> > >>>>>> > >>>>>> On 08/10/2015 01:05 PM, Joe Hershberger wrote: > >>>>>>> Hi York, > >>>>>>> > >>>>>>> On Mon, Aug 10, 2015 at 3:03 PM, York Sun > >>>>>>> wrote: > >>>>>>>> > >>>>>>>> > >>>>>>>> On 08/10/2015 12:57 PM, Joe Hershberger wrote: > >>>>>>>>> Too much top-posting. > >>>>>>>>> > >>>>>>>>> On Mon, Aug 10, 2015 at 2:41 PM, York Sun > wrote: > >>>>>>>>>> SPL doesn't use net/eth.c. You add a call in env_flags.c. > >>>>>>>>>> > >>>>>>>>>> I think you can put it in header file and use static inline, > >>>>>>>>>> or keep it in the same file where it is called. > >>>>>>>>> > >>>>>>>>> That is probably fine. > >>>>>>>>> > >>>>>>>>>> Another way is to undef CONFIG_CMD_NET for SPL part. It is > >>>>>>>>>> default to 'y' in Kconfig. Joe may have some good suggestion. > >>>>>>>>> > >>>>>>>>> I don't think this is the reason. The problem is that net is > >>>>>>>>> *not* build for SPL, but env is. > >>>>>>>> > >>>>>>>> Yes, env is built. The offending lines in common/env_flags.c > >>>>>>>> are gated by "#ifdef CONFIG_CMD_NET". That's why I say it could > >>>>>>>> be another > >>> way. > >>>>>>> > >>>>>>> OK, sure... but that breaks intended behavior, I think. > >>>>>>> > >>>>>> > >>>>>> I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. > >>>>>> So I guess the fix can be either to put the common function in > >>>>>> header file after making it really simple to reduce dependency, > >>>>>> or to keep the > >>>>> original code in env_flag.c. > >>>>>> > >>>>> > >>>>> Codrin, > >>>>> > >>>>> Can you prepare a new patch? You don't have to send the whole set. > >>>>> All but one have been acked by Joe. > >>>>> > >>>>> York > >>>> > >>>> I can't inline eth_validate_ethaddr_str in eth.h since it depends > >>>> on > >>> simple_strtoul and tolower. Also, I can't let it in > >>&g
