[U-Boot] [u-boot PATCH v4 04/10] ti: common: board_detect: commodify ethaddr environment setting code

[Roger Quadros]

On 13/03/17 15:15, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq at ti.com> writes:
>> +void board_ti_set_ethaddr(int index)
>> +{
>> +	uint8_t mac_addr[6];
>> +	int i;
>> +	u64 mac1, mac2;
>> +	u8 mac_addr1[6], mac_addr2[6];
>> +	int num_macs;
>> +	/*
>> +	 * Export any Ethernet MAC addresses from EEPROM.
>> +	 * The 2 MAC addresses in EEPROM define the address range.
>> +	 */
>> +	board_ti_get_eth_mac_addr(0, mac_addr1);
>> +	board_ti_get_eth_mac_addr(1, mac_addr2);
>> +
>> +	if (is_valid_ethaddr(mac_addr1) && is_valid_ethaddr(mac_addr2)) {
>> +		mac1 = mac_to_u64(mac_addr1);
>> +		mac2 = mac_to_u64(mac_addr2);
>> +
>> +		/* must contain an address range */
>> +		num_macs = mac2 - mac1 + 1;
>> +		/* <= 50 to protect against user programming error */
>> +		if (num_macs > 0 && num_macs <= 50) {
> 
> if user programs a range > 50, then you do nothing. How about allowing
> up to 50 addresses?
> 
> Something like:
> 
> 	if (num_macs < 0)
>         	bail_out();
> 
> 	if (num_macs > 50) {
> 		printf("Too many addresses. %d > 50\n", num_macs);
>                 num_macs = 50;
> 	}
> 
> this also mean you can reduce indentation level for the for loop.
> 

Good idea.

-- 
cheers,
-roger