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

[Sasha Khapyorsky]

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'.

> 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?

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