[ofa-general] Re: [PATCH] osm: Converting the the C++ code to C inosm_ucast_lash.c

[Yevgeny Kliteynik]

Hal Rosenstock wrote:
> On Wed, 2007-03-07 at 08:18, Michael S. Tsirkin wrote:
>>> Quoting Hal Rosenstock <halr at voltaire.com>:
>>> Subject: Re: [PATCH] osm: Converting the the C++ code to C inosm_ucast_lash.c
>>>
>>> On Wed, 2007-03-07 at 07:12, Yevgeny Kliteynik wrote:
>>>> Hal Rosenstock wrote:
>>>>> On Wed, 2007-03-07 at 03:40, Michael S. Tsirkin wrote:
>>>>>>> Quoting Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>:
>>>>>>> Subject: Re: [PATCH] osm: Converting the the C++ code to C in osm_ucast_lash.c
>>>>>>>
>>>>>>>
>>>>>>> Michael S. Tsirkin wrote:
>>>>>>>>> Hi Hal.
>>>>>>>>>
>>>>>>>>> Converting the the C++ code to C.
>>>>>>>>>
>>>>>>>>> Please apply both to trunk and to 1.2
>>>>>>>>>
>>>>>>>>> Thanks.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>
>>>>>>>> NAK.
>>>>>>>> 1. I don't see any C++ here.
>>>>>>>>
>>>>>>>> 2. Why do we need this on ofed branch?
>>>>>>>>    Only bugfixes should go there. What bug does it fix?
>>>>>>> There are 3 things in this patch:
>>>>>>> 1. int i -> uint16_t i
>>>>>>> 2. Moving variable declaration (switch_bitmap) to the beginning 
>>>>>>>    of the function (currently, it is declared after OSM_LOG_ENTER)
>>>>>>> 3. Changing C99 dynamically allocated array to the old style.
>>>>>>>
>>>>>>> First two can be categorized as bugs.
>>>>>>>
>>>>>>> The third one is for compiler on windows.
>>>>>>>
>>>>>>> Each of these elements breaks OSM compilation on Windows.
>>>>>>>
>>>>>>> If we don't include either of these, then OFED 1.2 OpenSM compilation
>>>>>>> on windows will be broken. 
>>>>>> Ultimately, whether to merge this this and where is up to the maintainer.  But I
>>>>>> note that OFED 1.2 goals do not include windows builds.
>>>>> While not a formal OFED 1.2 goal, doesn't this depend on whether there
>>>>> is intended to be a Windows equivalent to the OFED 1.2 OpenSM ?
>>>> I'm not aware of any plans for windows equivalent to the OFED 1.2 OpenSM,
>>> Should there be ?
>> Isn't that what we need to know to decide whether to merge this patch?
> 
> to OFED 1.2, yes.
> 
>>> master may be less stable and certainly is likely to
>>> be less tested than OFED 1.2 at any point in time.
>> I guess openib-windows guys will be able to branch off from ofed 1.2 branch
>> if they like. But even if you fix compilation issues on ofed 1.2 now, it's unlikely
>> a windows release won't include other changes as compared to the linux one.
> 
> I'm not sure what changes you are referring to but I would think the
> more tested the base is, the easier this is and fewer changes are
> involved.
> 
>> So why bother?
> 
> You're right that it's probably not worth the effort.

I checked it with the Windows team - they don't have any
intention to use OFED 1.2 OpenSM.

-- Yevgeny
 
> -- Hal
>