From: Doug M. <dma...@vm...> - 2011-04-12 21:16:57
|
On Apr 12, 2011, at 1:07 AM, Andrew Beekhof wrote: > On 04/11/2011 11:11 PM, Doug MacEachern wrote: >> Hi Andrew, >> >> ok, no problem, I have it building with the ms compiler now. > > Great > >> sigar_net_interface_name_get is declared extern, do you need that outside of the sigar library or can I make it static? > > We're not using it explicitly, so static should be fine. ok, cool. > >> >> +SIGAR_DECLARE(char *) >> +sigar_net_interface_name_get(sigar_t *sigar, MIB_IFROW *ifr) >> >> I'd also like to make PIP_ADAPTER_ADDRESSES an input to that function so we can avoid calling sigar_new each time. > > Sure. That makes sense. Just be careful how ifconf_buf is used - that > had us guess for a bit. Yeah, a bit too much reuse of that scratch buffer. > > Do you want us to work on this or have you got it under control? I got it for now. I want to spend a bit more time testing and get some mingw32 based CI builds hooked up, etc. Hopefully will be able to get that done soon. -Doug > >> -Doug >> >> On Apr 11, 2011, at 4:44 AM, Andrew Beekhof wrote: >> >>> On 04/08/2011 06:24 PM, Doug MacEachern wrote: >>>> Hey Guys, >>>> >>>> Apologies for the long delay.. >>>> >>>> On Mar 9, 2011, at 1:41 AM, Andrew Beekhof wrote: >>>> >>>>> On Wed, Mar 2, 2011 at 10:54 PM, Doug MacEachern<dma...@vm...> wrote: >>>>>> Hi Adam, >>>>>> >>>>>> Thanks for the patches! >>>>>> I need to take a closer look, but wondering if we could change sigar_net_interface_config_get rather than change the return values of sigar_net_interface_list: >>>>>> sigar_net_interface_config_get("ethX",&config); >>>>> >>>>> Until recently I probably would have agreed with you, however given >>>>> the recent shift away from ethX even on Linux, I'm not convinced that >>>>> it makes sense to continue using it on Windows where it has no real >>>>> meaning. Worse, the number of ethX entries does not even match the >>>>> number of physical devices (there are two additional "pseudo devices" >>>>> for every physical one). >>>>> >>>>> The other issue is that anything wanting to present network >>>>> information to users must do the translation - again, since ethX is >>>>> meaningless on Windows. >>>>> >>>>> So I guess my answer here is, "yes we could and if you insist we will, >>>>> but I really don't think we should" :-) >>>> >>>> I think your change is the right way to go for the long term. I just had current users in mind where this change may have an unexpected impact. In Hyperic HQ for example, ethX is used a the service name, so I believe this change would end up creating a new set of services there and the old ethX ones would just be stale. But, that's something that can be worked out.. we can leave the 1.6 branch as is and apply your changes to master/1.7. >>>> >>>> I'm just now trying out your patches, but it doesn't compile for me using VC 2005. I haven't looked into it yet, does it build for you with VC? >>> >>> We don't have access to VC, we've been using mingw32 instead. >>> But if you let us know what the errors are we'll do our best to fix them. >> > |