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

[Yevgeny Kliteynik]

Sasha Khapyorsky wrote:
> On Wed, 2007-03-07 at 10:22 +0200, Yevgeny Kliteynik wrote:
>> 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
> 
> Why it is needed at all? 'i' is compared with 'unsigned int', if you
> have signedness warnings you will want to change this to 'unsigned int'
> and not to 'uint16_t'.

Agree, "unsigned int" it is.
 
>> 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.
> 
> Is this breakage enforced by compilation flags?

No, by compiler that doesn't support all the C99 syntax yet.


Anyway, on second thought - no need to check it into the branch.
I'll resend the V2 of the patch.

-- Yevgeny
 
> Sasha
> 
>> If we don't include either of these, then OFED 1.2 OpenSM compilation
>> on windows will be broken. 
>>
>> -- Yevgeny
>>  
>>>> ---
>>>>  osm/opensm/osm_ucast_lash.c |   23 ++++++++++++++++-------
>>>>  1 files changed, 16 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/osm/opensm/osm_ucast_lash.c b/osm/opensm/osm_ucast_lash.c
>>>> index 0afa43c..8c9172e 100644
>>>> --- a/osm/opensm/osm_ucast_lash.c
>>>> +++ b/osm/opensm/osm_ucast_lash.c
>>>> @@ -406,7 +406,7 @@ static int get_phys_connection(switch_t
>>>>  static void shortest_path(lash_t *p_lash, int ir)
>>>>  {
>>>>    switch_t **switches = p_lash->switches, *sw, *swi;
>>>> -  int i;
>>>> +  uint16_t i;
>>>>    cl_list_t bfsq;
>>>>    
>>>>    cl_list_construct(&bfsq);
>>>> @@ -986,11 +986,18 @@ static int lash_core(lash_t *p_lash)
>>>>    int output_link2, i_next_switch2;
>>>>    int cycle_found2 = 0;
>>>>    int status = IB_SUCCESS;
>>>> +  int * switch_bitmap = NULL;
>>>>  
>>>>    OSM_LOG_ENTER(p_log, lash_core);
>>>>  
>>>> -  //Bitmap to check if we have processed this pair
>>>> -  int switch_bitmap[num_switches][num_switches];
>>>> +  switch_bitmap = (int *)malloc(num_switches * num_switches * sizeof(int));
>>>> +  if (!switch_bitmap)
>>>> +  {
>>>> +    osm_log(p_log, OSM_LOG_ERROR,
>>>> +            "lash_core: ERR 4D04: "
>>>> +            "Failed allocating switch_bitmap - out of memory\n");
>>>> +    goto Exit;
>>>> +  }
>>>>  
>>>>    for(i=0; i<num_switches; i++) {
>>>>      
>>>> @@ -1006,7 +1013,7 @@ static int lash_core(lash_t *p_lash)
>>>>     
>>>>      for(j=0; j<num_switches; j++) {
>>>>        for(k=0; k<num_switches; k++) {
>>>> -	switch_bitmap[j][k] = 0;
>>>> +	switch_bitmap[j * num_switches + k] = 0;
>>>>        }
>>>>        switches[j]->used_channels = 0;
>>>>        switches[j]->q_state = UNQUEUED;
>>>> @@ -1015,7 +1022,7 @@ static int lash_core(lash_t *p_lash)
>>>>    
>>>>    for(i=0; i<num_switches; i++) {
>>>>      for(dest_switch=0; dest_switch<num_switches; dest_switch++)
>>>> -      if(dest_switch != i && switch_bitmap[i][dest_switch] == 0) {
>>>> +      if(dest_switch != i && switch_bitmap[i * num_switches + dest_switch] == 0) {
>>>>  	v_lane = 0;
>>>>  	stop = 0;
>>>>  	while(v_lane < lanes_needed && stop == 0) {
>>>> @@ -1078,8 +1085,8 @@ static int lash_core(lash_t *p_lash)
>>>>  	  p_lash->virtual_location[i][dest_switch][v_lane] = 1;
>>>>  	  p_lash->virtual_location[dest_switch][i][v_lane] = 1;
>>>>  
>>>> -	  switch_bitmap[i][dest_switch] = 1;
>>>> -	  switch_bitmap[dest_switch][i] = 1;
>>>> +	  switch_bitmap[i * num_switches + dest_switch] = 1;
>>>> +	  switch_bitmap[dest_switch * num_switches + i] = 1;
>>>>  	}
>>>>  
>>>>        for(j=0; j<num_switches; j++) {
>>>> @@ -1115,6 +1122,8 @@ static int lash_core(lash_t *p_lash)
>>>>  	  "Lane requirements (%d) exceed available lanes (%d)\n",
>>>>  	  p_lash->vl_min, lanes_needed);
>>>>   Exit:
>>>> +  if (switch_bitmap)
>>>> +    free(switch_bitmap);
>>>>    OSM_LOG_EXIT(p_log);
>>>>    return status;
>>>>  }
>> _______________________________________________
>> general mailing list
>> general at lists.openfabrics.org
>> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>>
>> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
>