[U-Boot] [PATCH] net: Always build the string_to_enetaddr() helper
Ondřej Jirman
megous at megous.com
Sat Sep 14 14:05:44 UTC 2019
Hi,
On Fri, Sep 13, 2019 at 07:40:22PM -0500, Joe Hershberger wrote:
> Part of the env cleanup moved this out of the environment code and into
> the net code. However, this helper is sometimes needed even when the net
> stack isn't included.
>
> Move the helper to lib/net_utils.c like it's similarly-purposed
> string_to_ip(). Also rename the moved function to similar naming.
>
> Signed-off-by: Joe Hershberger <joe.hershberger at ni.com>
> Reported-by: Ondrej Jirman <megous at megous.com>
I've tested the patch and it works, but I'be found other related issue, where
u-boot thinks %pM will format a MAC address string, but it does just
print out the pointer due to relevant functions being gated by CONFIG_CMD_NET
guard in lib/vsprintf.c.
The gating should probably be done so that it panics/halts the u-boot if gated
pointer flags are used by u-boot code, because that will clearly be incorrect,
without calling code ever knowing. This way the user will know that something
is wrong and will have to fix the code.
I noticed be cause produced mac address is wrong.
Quick fix is to remove the CONFIG_CMD_NET gate.
regards,
o.
>
> ---
>
> arch/arm/mach-tegra/cboot.c | 2 +-
> board/renesas/sh7752evb/sh7752evb.c | 2 +-
> board/renesas/sh7753evb/sh7753evb.c | 2 +-
> board/renesas/sh7757lcr/sh7757lcr.c | 4 ++--
> cmd/ethsw.c | 2 +-
> cmd/nvedit.c | 2 +-
> doc/README.enetaddr | 4 ++--
> include/net.h | 24 +++++++++++++-----------
> lib/net_utils.c | 15 +++++++++++++++
> net/eth-uclass.c | 2 +-
> net/eth_legacy.c | 2 +-
> net/net.c | 12 ------------
> 12 files changed, 39 insertions(+), 34 deletions(-)
>
> diff --git a/arch/arm/mach-tegra/cboot.c b/arch/arm/mach-tegra/cboot.c
> index 0433081c6c..0762144ecf 100644
> --- a/arch/arm/mach-tegra/cboot.c
> +++ b/arch/arm/mach-tegra/cboot.c
> @@ -495,7 +495,7 @@ static int cboot_get_ethaddr_legacy(const void *fdt, uint8_t mac[ETH_ALEN])
> return -ENOENT;
> }
>
> - eth_parse_enetaddr(prop, mac);
> + string_to_enetaddr(prop, mac);
>
> if (!is_valid_ethaddr(mac)) {
> printf("Invalid MAC address: %s\n", prop);
> diff --git a/board/renesas/sh7752evb/sh7752evb.c b/board/renesas/sh7752evb/sh7752evb.c
> index d0b850f35d..b4292b22e7 100644
> --- a/board/renesas/sh7752evb/sh7752evb.c
> +++ b/board/renesas/sh7752evb/sh7752evb.c
> @@ -93,7 +93,7 @@ static void set_mac_to_sh_giga_eth_register(int channel, char *mac_string)
> unsigned char mac[6];
> unsigned long val;
>
> - eth_parse_enetaddr(mac_string, mac);
> + string_to_enetaddr(mac_string, mac);
>
> if (!channel)
> ether = GETHER0_MAC_BASE;
> diff --git a/board/renesas/sh7753evb/sh7753evb.c b/board/renesas/sh7753evb/sh7753evb.c
> index e1bed7dcc3..5aebb041b0 100644
> --- a/board/renesas/sh7753evb/sh7753evb.c
> +++ b/board/renesas/sh7753evb/sh7753evb.c
> @@ -100,7 +100,7 @@ static void set_mac_to_sh_giga_eth_register(int channel, char *mac_string)
> unsigned char mac[6];
> unsigned long val;
>
> - eth_parse_enetaddr(mac_string, mac);
> + string_to_enetaddr(mac_string, mac);
>
> if (!channel)
> ether = GETHER0_MAC_BASE;
> diff --git a/board/renesas/sh7757lcr/sh7757lcr.c b/board/renesas/sh7757lcr/sh7757lcr.c
> index d2671202e9..662c435f7a 100644
> --- a/board/renesas/sh7757lcr/sh7757lcr.c
> +++ b/board/renesas/sh7757lcr/sh7757lcr.c
> @@ -140,7 +140,7 @@ static void set_mac_to_sh_eth_register(int channel, char *mac_string)
> unsigned char mac[6];
> unsigned long val;
>
> - eth_parse_enetaddr(mac_string, mac);
> + string_to_enetaddr(mac_string, mac);
>
> if (!channel)
> ether = ETHER0_MAC_BASE;
> @@ -159,7 +159,7 @@ static void set_mac_to_sh_giga_eth_register(int channel, char *mac_string)
> unsigned char mac[6];
> unsigned long val;
>
> - eth_parse_enetaddr(mac_string, mac);
> + string_to_enetaddr(mac_string, mac);
>
> if (!channel)
> ether = GETHER0_MAC_BASE;
> diff --git a/cmd/ethsw.c b/cmd/ethsw.c
> index 8846805799..8d271ce1f3 100644
> --- a/cmd/ethsw.c
> +++ b/cmd/ethsw.c
> @@ -864,7 +864,7 @@ static int keyword_match_mac_addr(enum ethsw_keyword_id key_id, int argc,
> return 0;
> }
>
> - eth_parse_enetaddr(argv[*argc_nr + 1], parsed_cmd->ethaddr);
> + string_to_enetaddr(argv[*argc_nr + 1], parsed_cmd->ethaddr);
>
> if (is_broadcast_ethaddr(parsed_cmd->ethaddr)) {
> memset(parsed_cmd->ethaddr, 0xFF, sizeof(parsed_cmd->ethaddr));
> diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> index 1cb0bc1460..1e4b225a94 100644
> --- a/cmd/nvedit.c
> +++ b/cmd/nvedit.c
> @@ -360,7 +360,7 @@ ulong env_get_hex(const char *varname, ulong default_val)
>
> int eth_env_get_enetaddr(const char *name, uint8_t *enetaddr)
> {
> - eth_parse_enetaddr(env_get(name), enetaddr);
> + string_to_enetaddr(env_get(name), enetaddr);
> return is_valid_ethaddr(enetaddr);
> }
>
> diff --git a/doc/README.enetaddr b/doc/README.enetaddr
> index f926485986..5baa9f2179 100644
> --- a/doc/README.enetaddr
> +++ b/doc/README.enetaddr
> @@ -76,12 +76,12 @@ To assist in the management of these layers, a few helper functions exist. You
> should use these rather than attempt to do any kind of parsing/manipulation
> yourself as many common errors have arisen in the past.
>
> - * void eth_parse_enetaddr(const char *addr, uchar *enetaddr);
> + * void string_to_enetaddr(const char *addr, uchar *enetaddr);
>
> Convert a string representation of a MAC address to the binary version.
> char *addr = "00:11:22:33:44:55";
> uchar enetaddr[6];
> -eth_parse_enetaddr(addr, enetaddr);
> +string_to_enetaddr(addr, enetaddr);
> /* enetaddr now equals { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55 } */
>
> * int eth_env_get_enetaddr(char *name, uchar *enetaddr);
> diff --git a/include/net.h b/include/net.h
> index 75a16e4c8f..d054c5f694 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -825,6 +825,19 @@ static inline void net_random_ethaddr(uchar *addr)
> addr[0] |= 0x02; /* set local assignment bit (IEEE802) */
> }
>
> +/**
> + * string_to_enetaddr() - Parse a MAC address
> + *
> + * Convert a string MAC address
> + *
> + * Implemented in lib/net_utils.c (built unconditionally)
> + *
> + * @addr: MAC address in aa:bb:cc:dd:ee:ff format, where each part is a 2-digit
> + * hex value
> + * @enetaddr: Place to put MAC address (6 bytes)
> + */
> +void string_to_enetaddr(const char *addr, uint8_t *enetaddr);
> +
> /* Convert an IP address to a string */
> void ip_to_string(struct in_addr x, char *s);
>
> @@ -875,15 +888,4 @@ int update_tftp(ulong addr, char *interface, char *devstring);
>
> /**********************************************************************/
>
> -/**
> - * eth_parse_enetaddr() - Parse a MAC address
> - *
> - * Convert a string MAC address
> - *
> - * @addr: MAC address in aa:bb:cc:dd:ee:ff format, where each part is a 2-digit
> - * hex value
> - * @enetaddr: Place to put MAC address (6 bytes)
> - */
> -void eth_parse_enetaddr(const char *addr, uint8_t *enetaddr);
> -
> #endif /* __NET_H__ */
> diff --git a/lib/net_utils.c b/lib/net_utils.c
> index 9fb9d4a4b0..ed5044c3de 100644
> --- a/lib/net_utils.c
> +++ b/lib/net_utils.c
> @@ -41,3 +41,18 @@ struct in_addr string_to_ip(const char *s)
> addr.s_addr = htonl(addr.s_addr);
> return addr;
> }
> +
> +void string_to_enetaddr(const char *addr, uint8_t *enetaddr)
> +{
> + char *end;
> + int i;
> +
> + if (!enetaddr)
> + return;
> +
> + for (i = 0; i < 6; ++i) {
> + enetaddr[i] = addr ? simple_strtoul(addr, &end, 16) : 0;
> + if (addr)
> + addr = (*end) ? end + 1 : end;
> + }
> +}
> diff --git a/net/eth-uclass.c b/net/eth-uclass.c
> index 3bd98b01ad..9fe4096120 100644
> --- a/net/eth-uclass.c
> +++ b/net/eth-uclass.c
> @@ -227,7 +227,7 @@ static int on_ethaddr(const char *name, const char *value, enum env_op op,
> switch (op) {
> case env_op_create:
> case env_op_overwrite:
> - eth_parse_enetaddr(value, pdata->enetaddr);
> + string_to_enetaddr(value, pdata->enetaddr);
> eth_write_hwaddr(dev);
> break;
> case env_op_delete:
> diff --git a/net/eth_legacy.c b/net/eth_legacy.c
> index 41f5263526..5d6b0d7d7f 100644
> --- a/net/eth_legacy.c
> +++ b/net/eth_legacy.c
> @@ -117,7 +117,7 @@ static int on_ethaddr(const char *name, const char *value, enum env_op op,
> switch (op) {
> case env_op_create:
> case env_op_overwrite:
> - eth_parse_enetaddr(value, dev->enetaddr);
> + string_to_enetaddr(value, dev->enetaddr);
> eth_write_hwaddr(dev, "eth", dev->index);
> break;
> case env_op_delete:
> diff --git a/net/net.c b/net/net.c
> index ded86e7456..4d2b7ead3b 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1628,15 +1628,3 @@ ushort env_get_vlan(char *var)
> {
> return string_to_vlan(env_get(var));
> }
> -
> -void eth_parse_enetaddr(const char *addr, uint8_t *enetaddr)
> -{
> - char *end;
> - int i;
> -
> - for (i = 0; i < 6; ++i) {
> - enetaddr[i] = addr ? simple_strtoul(addr, &end, 16) : 0;
> - if (addr)
> - addr = (*end) ? end + 1 : end;
> - }
> -}
> --
> 2.11.0
>
More information about the U-Boot
mailing list