[U-Boot] [PATCH v2 2/2] lib: net_utils: enforce '.' as octet separator in string_to_ip

[Chris Packham]

On Mon, Jan 9, 2017 at 7:45 AM, Wolfgang Denk <wd at denx.de> wrote:
> Dear Chris,
>
> In message <CAFOYHZDK+cOs-1ODjSCBF7OTERTzMgBgBhRC+WSRG4C5iToB+A at mail.gmail.com> you wrote:
>>
>> > The old code was forgiving and would accept 192,168,1,2 as well.
>>
>> Technically you can't enter that. The env_flags.c code prevents that
>> from being added to environment variables that have been tagged as ip
>> addresses. These patches are pushing the logic down a bit further. The
>> code handling env_flags_vartype_ipaddr could be updated to use
>> string_to_ip instead.
>
> I'm not 100% sure about that.  U-Boot environment is a complex thing.
> For example, what happens if we use "env import" to import variable
> settings from text or binary formats?  Are all these checks present
> then, too?  [Sorry for asking, I really don't know.]
>

I can say for sure either. What I can say is that some checking
already exists so technically this is duplicating functionality. The
change to actually replace one with the other would need to consider
these cases that you've raise.

>> > Also, at least crtitical parts of the network code (NFS, TFTP) do not
>> > check the return value of string_to_ip() - so what is the benefit of
>> > this change?
>> >
>>
>> The reasoning behind this change is to prepare for parsing ipv6
>> addresses, which can contain ipv4 format addresses provided they are
>> at the end.
>>
>> e.g. This is a valid ipv6 address "::ffff:192.168.1.1" and so is
>> "::ffff:0192:0168:0001:0001" but the former triggers the ipv4 mapping
>> logic.
>
> Ah, I see.  Makes sense.
>
> Should we add error handling to TFTP and NFS, then?
>

I think the "checking" I'm referring to in net_check_prereq() seems to
cover the bases for now. If we want to distinguish between invalid,
set to 0.0.0.0 and not set then we'd need to start adding additional
error handling.