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

Michael McMahon michael.x.mcmahon at oracle.com
Fri Feb 25 09:22:30 PST 2011


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.

- Michael



More information about the net-dev mailing list