Code Review 7022269: clean up fscanf usage in soalris networking native code

[Michael McMahon]

Chris Hegarty wrote:
> On 02/25/11 05:22 PM, Michael McMahon wrote:
>> Chris Hegarty wrote:
>>> On 02/25/11 02:39 PM, Michael McMahon wrote:
>>>> Alan Bateman wrote:
>>>>> Chris Hegarty wrote:
>>>>>> Michael, Alan,
>>>>>>
>>>>>> Some small cleanups to the use of scanf with specified string width.
>>>>>> The buffers should be sized large enough to handle the specified
>>>>>> width and the null terminator.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~chegar/7022269/webrev.00/webrev/
>>>>>>
>>>>>> -Chris.
>>>>> Looks okay although slightly inconsistent in that the code in
>>>>> initLocalIfs allows a device name up to 32 chars but the other places
>>>>> it is 20. I don't know what the right answer is.
>>>>>
>>>>> -Alan
>>>> Would it be better to #define a constant and use constant+1 then where
>>>> the array
>>>> is allocated. Maybe use the same 32 byte storage in all places.
>>>> Not a big deal really.
>>>
>>> In net/ipv6/addrconf.c (kernel source) they use %8s, but then size the
>>> char array using IFNAMSIZ (16). Adding a new definition for the sizing
>>> may be a little misleading. We should probably use these values, but
>>> then I'd be nervous about making any reductions in size.
>>>
>>> I guess one issue is that the string width in scanf is always going to
>>> be a hardcoded value no matter what definitions you make. And that
>>> value has a direct impact on the array sizing.
>>>
>>> It's common practice to use scanf ( and its variants ) with defined
>>> string widths and arrays. You just need to be aware of the potential
>>> overflows. For this reason I don't see any value in a #define.
>>>
>> Yeah, I was thinking you could do something clever with C string
>> concatenation and/or
>> string macros. But, I don't think there is an easy way to do it. Never
>> mind.
>
> Are you ok with the change as is?
>
Yes.
> -Chris.
>
>>
>> - Michael