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 Apr 18, 2019, at 2:32 AM, 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
It might be nice to use __BIT(), __SHIFT*() etc. macros (if they can do ULL constants)... I think it would make all of the constants a lot easier to read.
I suppose we could add an ifmword_t ... but if we have to enlarge it again, it's going to become a struct (or something else entirely), right? I guess it makes the "grep" phase of the next overhaul a little easier :-)
>
> 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)
>
>
> Thanks.
>
> --
> -----------------------------------------------
> SAITOH Masanobu (msaitoh%execsw.org@localhost
> msaitoh%netbsd.org@localhost)
-- thorpej
Home |
Main Index |
Thread Index |
Old Index