tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: change ifmedia word from int to uint64_t



On 2019/04/18 20:43, Christos Zoulas wrote:
> In article <aa73b776-930f-2c6d-a7e3-f68f865d9674%execsw.org@localhost>,
> Masanobu SAITOH  <msaitoh%execsw.org@localhost> wrote:
>> Hi.
>>
>> I'm now working to extend ifmedia word to be able to support
>> more than 10Gbps media.
>>
>> Patch:
>>
>> 	http://www.netbsd.org/~msaitoh/ifmedia64-20190418-0.dif
>>
>> I changed ifmedia's word from int to uint64_t. It's almost the
>> same as OpenBSD who did 5 years ago.
>>
>> What do you think about the above diff?
>> IMHO, it would be better to change ifmword_t(with uint64_t)
>> than uint64_t. For example
>>
>> (from ifconfig/media.c's diff)
>>> @@ -374,7 +374,8 @@ media_status(prop_dictionary_t env, prop
>>>  	/* Interface link status is queried through SIOCGIFMEDIA.
>>>  	 * Not all interfaces have actual media. */
>>>  	if (ifmr.ifm_count != 0) {
>>> -		media_list = (int *)malloc(ifmr.ifm_count * sizeof(int));
>>> +		media_list = (uint64_t *)malloc(ifmr.ifm_count
>>> +		    * sizeof(*media_list));
>>>  		if (media_list == NULL)
>>>  			err(EXIT_FAILURE, "malloc");
>>>  		ifmr.ifm_ulist = media_list;
>>
>> If we use ifmword_t, it makes easy to change the size in future.
>>
>>
>> This patch has some TODOs:
>>
>> - COMPAT_NETBSD32? (currently i386 old binary doesn't work on
>>   amd64)
>> - I would change the hook point. Current location is not good
>>   because we have to add "case: OSIOC[GS]IFMEDIA" to all driver
>>   which calls ifmedia_ioctl(). (see if_wm.c's patch)
> 
> - Casts to malloc are harmful.
> - malloc with multiplication should be converted to calloc which
>   checks for overflow.

Done in -current.

Thanks!

> christos
> 


-- 
-----------------------------------------------
                SAITOH Masanobu (msaitoh%execsw.org@localhost
                                 msaitoh%netbsd.org@localhost)


Home | Main Index | Thread Index | Old Index