Re: [PATCH] ofwboot bootp/bootparams mixup

[Daniele Forsi <dforsi%gmail.com@localhost>]

2009/5/4 Roy Marples>:
> Christos Zoulas wrote:
>> In article <49FEC7F2.1070708%marples.name@localhost>,
>>>              }
>>> -            if (tag == TAG_ROOTPATH) {
>>> +            if (tag == TAG_ROOTPATH && size <= sizeof(rootpath)) {
>>>                      strncpy(rootpath, (char *)cp, sizeof(rootpath));
>>>                      rootpath[size] = '\0';
>>>              }
>>> -            if (tag == TAG_HOSTNAME) {
>>> +            if (tag == TAG_HOSTNAME && size <= sizeof(hostname)) {
>>>                      strncpy(hostname, (char *)cp, sizeof(hostname));
>>>                      hostname[size] = '\0';
>>
>> The <= test should really be < I think.
>
> Yes, good catch.

isn't it redundant checking size < sizeof(hostname) before using
strncpy() with the same argument and terminating the string? In that
case the string is always terminated.
How about not checking size < sizeof(hostname) and replacing
hostname[size] = '\0'; with hostname[sizeof(hostname) - 1] = '\0';

> -               if (tag == TAG_SWAPSERVER) {
> +               if (tag == TAG_SWAPSERVER && size >= sizeof(rootip.s_addr)) {
>                        /* let it override bp_siaddr */
>                        (void)memcpy(&rootip.s_addr, cp, 
> sizeof(rootip.s_addr));
>                }

I might be missing something, but why copying when size is bigger?
Shouldn't it be <= ?

-- 
Daniele Forsi