Re: [U-Boot] [PATCH 09/11] net: Apply default format rules to all ethaddr

2015-04-27 Thread Simon Glass
Hi Joe,

On 27 April 2015 at 13:35, Joe Hershberger  wrote:
> Hi Simon,
>
> On Thu, Apr 23, 2015 at 11:34 PM, Simon Glass  wrote:
>> On 21 April 2015 at 16:02, Joe Hershberger  wrote:
>>> Use a regular expression to apply the default formatting flags for all
>>> ethaddr env vars.
>>>
>>> Signed-off-by: Joe Hershberger 
>>> ---
>>>
>>>  include/env_flags.h | 11 ---
>>>  test/dm/eth.c   |  1 +
>>>  2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> Reviewed-by: Simon Glass 
>>
>> Q below.
>>
>>>
>>> diff --git a/include/env_flags.h b/include/env_flags.h
>>> index 3ef6311..fc6d0d8 100644
>>> --- a/include/env_flags.h
>>> +++ b/include/env_flags.h
>>> @@ -38,13 +38,18 @@ enum env_flags_varaccess {
>>>  #endif
>>>
>>>  #ifdef CONFIG_CMD_NET
>>> +#ifdef CONFIG_REGEX
>>> +#define ETHADDR_WILDCARD "\\d?"
>>> +#else
>>> +#define ETHADDR_WILDCARD
>>> +#endif
>>>  #ifdef CONFIG_ENV_OVERWRITE
>>> -#define ETHADDR_FLAGS "ethaddr:ma,"
>>> +#define ETHADDR_FLAGS "eth" ETHADDR_WILDCARD "addr:ma,"
>>>  #else
>>>  #ifdef CONFIG_OVERWRITE_ETHADDR_ONCE
>>> -#define ETHADDR_FLAGS "ethaddr:mc,"
>>> +#define ETHADDR_FLAGS "eth" ETHADDR_WILDCARD "addr:mc,"
>>>  #else
>>> -#define ETHADDR_FLAGS "ethaddr:mo,"
>>> +#define ETHADDR_FLAGS "eth" ETHADDR_WILDCARD "addr:mo,"
>>>  #endif
>>>  #endif
>>>  #else
>>> diff --git a/test/dm/eth.c b/test/dm/eth.c
>>> index 4891f3a..9b714a1 100644
>>> --- a/test/dm/eth.c
>>> +++ b/test/dm/eth.c
>>> @@ -89,6 +89,7 @@ static int dm_test_eth_rotate(struct dm_test_state *dms)
>>> /* Invalidate eth1's MAC address */
>>> net_ping_ip = string_to_ip("1.1.2.2");
>>> strcpy(ethaddr, getenv("eth1addr"));
>>
>> Can you explain this next line, please?
>>
>>> +   setenv(".flags", "eth1addr");
>
> This is now needed to allow the eth1addr to be modified. The env
> variable ".flags" overrides the static list that is compiled in. Since
> the regex now applies the ethaddr rules to eth\d?addr all of the
> ethaddr vars are restricted to write-once by default. You'll notice in
> another test where this was already overridden for "ethaddr" since it
> already had the flags applied.
>
> This line has the effect of changing the flags for that variable to be
> "default" (i.e. unrestricted).

OK I see. Would you mind adding this comment here in the code?

>
>>> setenv("eth1addr", NULL);
>>>
>>> /* Make sure that the default is to rotate to the next interface */
>>> --
>>> 1.7.11.5

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 09/11] net: Apply default format rules to all ethaddr

2015-04-27 Thread Joe Hershberger
Hi Simon,

On Thu, Apr 23, 2015 at 11:34 PM, Simon Glass  wrote:
> On 21 April 2015 at 16:02, Joe Hershberger  wrote:
>> Use a regular expression to apply the default formatting flags for all
>> ethaddr env vars.
>>
>> Signed-off-by: Joe Hershberger 
>> ---
>>
>>  include/env_flags.h | 11 ---
>>  test/dm/eth.c   |  1 +
>>  2 files changed, 9 insertions(+), 3 deletions(-)
>
> Reviewed-by: Simon Glass 
>
> Q below.
>
>>
>> diff --git a/include/env_flags.h b/include/env_flags.h
>> index 3ef6311..fc6d0d8 100644
>> --- a/include/env_flags.h
>> +++ b/include/env_flags.h
>> @@ -38,13 +38,18 @@ enum env_flags_varaccess {
>>  #endif
>>
>>  #ifdef CONFIG_CMD_NET
>> +#ifdef CONFIG_REGEX
>> +#define ETHADDR_WILDCARD "\\d?"
>> +#else
>> +#define ETHADDR_WILDCARD
>> +#endif
>>  #ifdef CONFIG_ENV_OVERWRITE
>> -#define ETHADDR_FLAGS "ethaddr:ma,"
>> +#define ETHADDR_FLAGS "eth" ETHADDR_WILDCARD "addr:ma,"
>>  #else
>>  #ifdef CONFIG_OVERWRITE_ETHADDR_ONCE
>> -#define ETHADDR_FLAGS "ethaddr:mc,"
>> +#define ETHADDR_FLAGS "eth" ETHADDR_WILDCARD "addr:mc,"
>>  #else
>> -#define ETHADDR_FLAGS "ethaddr:mo,"
>> +#define ETHADDR_FLAGS "eth" ETHADDR_WILDCARD "addr:mo,"
>>  #endif
>>  #endif
>>  #else
>> diff --git a/test/dm/eth.c b/test/dm/eth.c
>> index 4891f3a..9b714a1 100644
>> --- a/test/dm/eth.c
>> +++ b/test/dm/eth.c
>> @@ -89,6 +89,7 @@ static int dm_test_eth_rotate(struct dm_test_state *dms)
>> /* Invalidate eth1's MAC address */
>> net_ping_ip = string_to_ip("1.1.2.2");
>> strcpy(ethaddr, getenv("eth1addr"));
>
> Can you explain this next line, please?
>
>> +   setenv(".flags", "eth1addr");

This is now needed to allow the eth1addr to be modified. The env
variable ".flags" overrides the static list that is compiled in. Since
the regex now applies the ethaddr rules to eth\d?addr all of the
ethaddr vars are restricted to write-once by default. You'll notice in
another test where this was already overridden for "ethaddr" since it
already had the flags applied.

This line has the effect of changing the flags for that variable to be
"default" (i.e. unrestricted).

>> setenv("eth1addr", NULL);
>>
>> /* Make sure that the default is to rotate to the next interface */
>> --
>> 1.7.11.5
>>
>
> Regards,
> Simon
> ___
> 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 09/11] net: Apply default format rules to all ethaddr

2015-04-24 Thread Simon Glass
On 21 April 2015 at 16:02, Joe Hershberger  wrote:
> Use a regular expression to apply the default formatting flags for all
> ethaddr env vars.
>
> Signed-off-by: Joe Hershberger 
> ---
>
>  include/env_flags.h | 11 ---
>  test/dm/eth.c   |  1 +
>  2 files changed, 9 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass 

Q below.

>
> diff --git a/include/env_flags.h b/include/env_flags.h
> index 3ef6311..fc6d0d8 100644
> --- a/include/env_flags.h
> +++ b/include/env_flags.h
> @@ -38,13 +38,18 @@ enum env_flags_varaccess {
>  #endif
>
>  #ifdef CONFIG_CMD_NET
> +#ifdef CONFIG_REGEX
> +#define ETHADDR_WILDCARD "\\d?"
> +#else
> +#define ETHADDR_WILDCARD
> +#endif
>  #ifdef CONFIG_ENV_OVERWRITE
> -#define ETHADDR_FLAGS "ethaddr:ma,"
> +#define ETHADDR_FLAGS "eth" ETHADDR_WILDCARD "addr:ma,"
>  #else
>  #ifdef CONFIG_OVERWRITE_ETHADDR_ONCE
> -#define ETHADDR_FLAGS "ethaddr:mc,"
> +#define ETHADDR_FLAGS "eth" ETHADDR_WILDCARD "addr:mc,"
>  #else
> -#define ETHADDR_FLAGS "ethaddr:mo,"
> +#define ETHADDR_FLAGS "eth" ETHADDR_WILDCARD "addr:mo,"
>  #endif
>  #endif
>  #else
> diff --git a/test/dm/eth.c b/test/dm/eth.c
> index 4891f3a..9b714a1 100644
> --- a/test/dm/eth.c
> +++ b/test/dm/eth.c
> @@ -89,6 +89,7 @@ static int dm_test_eth_rotate(struct dm_test_state *dms)
> /* Invalidate eth1's MAC address */
> net_ping_ip = string_to_ip("1.1.2.2");
> strcpy(ethaddr, getenv("eth1addr"));

Can you explain this next line, please?

> +   setenv(".flags", "eth1addr");
> setenv("eth1addr", NULL);
>
> /* Make sure that the default is to rotate to the next interface */
> --
> 1.7.11.5
>

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot