[openib-general] [PATCH] opensm: updn: make local functions static + local types

Hal Rosenstock halr at voltaire.com
Mon Oct 23 02:13:49 PDT 2006


Eitan,

On Mon, 2006-10-23 at 03:20, Eitan Zahavi wrote:
> Hi Sasha,
> 
> If we would like to change OpenSM coding style to not include __osm 
> prefix for
> all static functions we should do it all over the code.

Is there any value to __osm_ in the local function names ? If not, I
don't really see the harm here.

> Meanwhile lets keep the style as it is. I thought we all agreed to this 
> in the past.
> It does not make sense to me to have a creeping style change one for 
> every developer involved.
> 
> Should we start the thread for what should be our target style and 
> convert all files now?
> If we do then lets agree on that - and then change.

Do all such changes need to be hung on the yet to be determined coding
style ?

-- Hal

> Thanks
> 
> Eitan
> 
> Sasha Khapyorsky wrote:
> > On 10:53 Sun 22 Oct     , Yevgeny Kliteynik wrote:
> >   
> >> Hi Sasha.
> >>
> >> One small comments:
> >>
> >> [snip]
> >>     
> >>>  osm_updn_find_root_nodes_by_min_hop(OUT updn_t *p_updn);
> >>>  ...
> >>>  osm_updn_find_root_nodes_by_min_hop(
> >>>  ...
> >>>  osm_subn_set_up_down_min_hop_table(
> >>>  ...
> >>>  osm_subn_calc_up_down_min_hop_table(
> >>>  ...
> >>>       
> >>    
> >> Please add the "__" prefix to the static function names.
> >>     
> >
> > Then would be better to remove 'osm_' and '__osm_' prefixes in static
> > names, but this will be function renaming, not just 'make static'.
> >
> > Sasha
> >
> >   
> >> Thanks.
> >>
> >> --
> >> Yevgeny
> >>
> >> Sasha Khapyorsky wrote:
> >>     
> >>> This makes local functions static and moves definitions of locally used
> >>> types to .c file.
> >>>
> >>> Signed-off-by: Sasha Khapyorsky <sashak at voltaire.com>
> >>> ---
> >>>  osm/include/opensm/osm_opensm.h     |    1 -
> >>>  osm/include/opensm/osm_ucast_updn.h |  349 -----------------------------------
> >>>  osm/opensm/osm_ucast_updn.c         |   81 +++++++-
> >>>  3 files changed, 70 insertions(+), 361 deletions(-)
> >>>
> >>> diff --git a/osm/include/opensm/osm_opensm.h b/osm/include/opensm/osm_opensm.h
> >>> index cb216a4..5557dbd 100644
> >>> --- a/osm/include/opensm/osm_opensm.h
> >>> +++ b/osm/include/opensm/osm_opensm.h
> >>> @@ -62,7 +62,6 @@ #include <opensm/osm_db.h>
> >>>  #include <opensm/osm_subnet.h>
> >>>  #include <opensm/osm_mad_pool.h>
> >>>  #include <opensm/osm_vl15intf.h>
> >>> -#include <opensm/osm_ucast_updn.h>
> >>>  
> >>>  #ifdef __cplusplus
> >>>  #  define BEGIN_C_DECLS extern "C" {
> >>> diff --git a/osm/include/opensm/osm_ucast_updn.h b/osm/include/opensm/osm_ucast_updn.h
> >>> index 4609e1b..c2a4376 100644
> >>> --- a/osm/include/opensm/osm_ucast_updn.h
> >>> +++ b/osm/include/opensm/osm_ucast_updn.h
> >>> @@ -71,363 +71,14 @@ BEGIN_C_DECLS
> >>>  /*  ENUM TypeDefs */
> >>>  /* /////////////////////////// */
> >>>  
> >>> -/*
> >>> -* DESCRIPTION
> >>> -*       This enum respresent available directions of arcs in the graph
> >>> -* SYNOPSIS
> >>> -*/
> >>> -typedef enum _updn_switch_dir
> >>> -{
> >>> -    UP = 0,
> >>> -    DOWN
> >>> -} updn_switch_dir_t;
> >>> -
> >>> -/*
> >>> - * TYPE DEFINITIONS
> >>> - *    UP 
> >>> - *      Current switch direction in propogating the subnet is up
> >>> - *    DOWN 
> >>> - *      Current switch direction in propogating the subnet is down
> >>> - *
> >>> - */
> >>> -
> >>> -/*
> >>> -* DESCRIPTION
> >>> -*       This enum respresent available states in the UPDN algorithm
> >>> -* SYNOPSIS
> >>> -*/
> >>> -typedef enum _updn_state
> >>> -{
> >>> -    UPDN_INIT = 0,
> >>> -    UPDN_RANK,
> >>> -    UPDN_MIN_HOP_CALC,
> >>> -} updn_state_t;
> >>> -
> >>> -/*
> >>> - * TYPE DEFINITIONS
> >>> - * UPDN_INIT - loading the package but still not performing anything
> >>> - * UPDN_RANK - post ranking algorithm
> >>> - * UPDN_MIN_HOP_CALC - post min hop table calculation
> >>> - */
> >>> -
> >>>  /* ////////////////////////////////// */
> >>>  /*  Struct TypeDefs */
> >>>  /* ///////////////////////////////// */
> >>>  
> >>> -/****s* UPDN: Rank element/updn_rank_t
> >>> -* NAME
> >>> -*	updn_rank_t
> >>> -*
> >>> -* DESCRIPTION
> >>> -*	This object represents a rank type element in a list
> >>> -*
> >>> -*	The updn_rank_t object should be treated as opaque and should
> >>> -*	be manipulated only through the provided functions.
> >>> -*
> >>> -* SYNOPSIS
> >>> -*/
> >>> -
> >>> -typedef struct _updn_rank
> >>> -{
> >>> -  cl_map_item_t map_item;
> >>> -  uint8_t rank;
> >>> -} updn_rank_t;
> >>> -
> >>> -/*
> >>> -* FIELDS
> >>> -*	map_item
> >>> -*		Linkage structure for cl_qmap.  MUST BE FIRST MEMBER!
> >>> -*
> >>> -*	rank
> >>> -*		Rank value of this node
> >>> -*
> >>> -*/
> >>> -
> >>> -/****s* UPDN: Histogram element/updn_hist_t
> >>> -* NAME
> >>> -*	updn_hist_t
> >>> -*
> >>> -* DESCRIPTION
> >>> -*	This object represents a histogram type element in a list
> >>> -*
> >>> -*	The updn_hist_t object should be treated as opaque and should
> >>> -*	be manipulated only through the provided functions.
> >>> -*
> >>> -* SYNOPSIS
> >>> -*/
> >>> -
> >>> -typedef struct _updn_hist
> >>> -{
> >>> -  cl_map_item_t map_item;
> >>> -  uint32_t bar_value;
> >>> -} updn_hist_t;
> >>> -
> >>> -/*
> >>> -* FIELDS
> >>> -*	map_item
> >>> -*		Linkage structure for cl_qmap.  MUST BE FIRST MEMBER!
> >>> -*
> >>> -*	bar_value
> >>> -*		The number of occurences of the same hop value 
> >>> -*
> >>> -*/
> >>> -
> >>> -typedef struct _updn_next_step
> >>> -{
> >>> -  updn_switch_dir_t state;
> >>> -  osm_switch_t *p_sw;
> >>> -} updn_next_step_t;
> >>> -
> >>> -/*****s* updn: updn/updn_input_t
> >>> -* NAME  updn_t
> >>> -*       
> >>> -*
> >>> -* DESCRIPTION
> >>> -*       updn input fields structure.
> >>> -*
> >>> -* SYNOPSIS
> >>> -*/
> >>> -
> >>> -typedef struct _updn_input
> >>> -{
> >>> -  uint32_t num_guids;
> >>> -  uint64_t * guid_list;
> >>> -} updn_input_t;
> >>> -
> >>> -/*
> >>> -* FIELDS
> >>> -*       num_guids
> >>> -*            number of guids given at the UI
> >>> -*
> >>> -*       guid_list
> >>> -*            guids specified as an array (converted from a list given in the UI)
> >>> -*
> >>> -*
> >>> -* SEE ALSO
> >>> -*      
> >>> -*********/
> >>> -
> >>> -/*****s* updn: updn/updn_t
> >>> -* NAME  updn_t
> >>> -*       
> >>> -*
> >>> -* DESCRIPTION
> >>> -*       updn structure.
> >>> -*
> >>> -* SYNOPSIS
> >>> -*/
> >>> -
> >>> -typedef struct _updn
> >>> -{
> >>> -  updn_state_t   state;
> >>> -  boolean_t      auto_detect_root_nodes;
> >>> -  cl_qmap_t      guid_rank_tbl;
> >>> -  updn_input_t   updn_ucast_reg_inputs;
> >>> -  cl_list_t *    p_root_nodes;
> >>> -} updn_t;
> >>> -
> >>> -/*
> >>> -* FIELDS
> >>> -*       state
> >>> -*            state of the updn algorithm which basically should pass through Init 
> >>> -*            - Ranking - UpDn algorithm
> >>> -*
> >>> -*       guid_rank_tbl
> >>> -*            guid 2 rank mapping vector , indexed by guid in network order
> >>> -*
> >>> -*
> >>> -* SEE ALSO
> >>> -*      
> >>> -*********/
> >>> -
> >>>  /* ////////////////////////////// */
> >>>  /*  Function  */
> >>>  /* ////////////////////////////// */
> >>>  
> >>> -/***f** OpenSM: Updn/updn_construct
> >>> -* NAME
> >>> -*       updn_construct
> >>> -*
> >>> -* DESCRIPTION
> >>> -*      Allocation of updn_t struct
> >>> -*
> >>> -* SYNOPSIS
> >>> -*/
> >>> -
> >>> -updn_t*
> >>> -updn_construct(void);
> >>> -
> >>> -/*
> >>> -* PARAMETERS
> >>> -*
> >>> -*
> >>> -* RETURN VALUE
> >>> -*       Return a pointer to an updn struct. Null if fails to do so.
> >>> -*
> >>> -* NOTES
> >>> -*       First step of the creation of updn_t
> >>> -*/
> >>> -
> >>> -/****s* OpenSM: Updn/updn_destroy
> >>> -* NAME
> >>> -*       updn_destroy
> >>> -*
> >>> -* DESCRIPTION
> >>> -*      release of updn_t struct
> >>> -*
> >>> -* SYNOPSIS
> >>> -*/
> >>> -
> >>> -void
> >>> -updn_destroy(
> >>> -  IN updn_t* const p_updn );
> >>> -
> >>> -/*
> >>> -* PARAMETERS
> >>> -*       p_updn
> >>> -*               A pointer to the updn_t struct that is goining to be released
> >>> -*
> >>> -* RETURN VALUE
> >>> -*      
> >>> -* NOTES
> >>> -*       Final step of the releasing of updn_t
> >>> -*
> >>> -* SEE ALSO
> >>> -*       updn_construct
> >>> -*********/
> >>> -
> >>> -/****f* OpenSM: Updn/updn_init
> >>> -* NAME
> >>> -*       updn_init
> >>> -*
> >>> -* DESCRIPTION
> >>> -*      Initialization of an updn_t struct
> >>> -*
> >>> -* SYNOPSIS
> >>> -*/
> >>> -cl_status_t
> >>> -updn_init(
> >>> -  IN updn_t* const p_updn );
> >>> -
> >>> -/*
> >>> -* PARAMETERS
> >>> -*       p_updn
> >>> -*               A pointer to the updn_t struct that is goining to be initilized
> >>> -*
> >>> -* RETURN VALUE
> >>> -*       The status of the function.
> >>> -*      
> >>> -* NOTES
> >>> -*       
> >>> -* SEE ALSO
> >>> -*       updn_construct
> >>> -********/
> >>> -
> >>> -/****** OpenSM: Updn/updn_subn_rank
> >>> -* NAME
> >>> -*	updn_subn_rank
> >>> -*
> >>> -* DESCRIPTION
> >>> -*	This function ranks the subnet for credit loop free algorithm
> >>> -*
> >>> -* SYNOPSIS
> >>> -*/
> >>> -int
> >>> -updn_subn_rank(
> >>> -	      IN uint64_t  root_guid ,
> >>> -	      IN uint8_t base_rank,
> >>> -	      IN updn_t* p_updn );
> >>> -
> >>> -/*
> >>> -* PARAMETERS
> >>> -*	p_subn
> >>> -*		[in] Pointer to a Subnet object to construct.
> >>> -*
> >>> -*       base_rank
> >>> -*		[in] The base ranking value (lowest value)
> >>> -*
> >>> -*	p_updn
> >>> -*		[in] Pointer to updn structure which includes state & lid2rank table
> >>> -*
> >>> -* RETURN VALUE
> >>> -*	This function returns 0 when rankning has succeded , otherwise 1.
> >>> -******/
> >>> -
> >>> -/****** OpenSM: UpDn/osm_subn_set_up_down_min_hop_table
> >>> -* NAME
> >>> -*	osm_subn_set_up_down_min_hop_table
> >>> -*
> >>> -* DESCRIPTION
> >>> -*	This function set min hop table of all switches by BFS through each
> >>> -*       port guid at the subnet using ranking done before.
> >>> -*
> >>> -* SYNOPSIS
> >>> -*/
> >>> -
> >>> -int
> >>> -osm_subn_set_up_down_min_hop_table(
> >>> -	      IN updn_t* p_updn );
> >>> -
> >>> -/*
> >>> -* PARAMETERS
> >>> -*	p_updn
> >>> -*		[in] Pointer to updn structure which includes state & lid2rank table
> >>> -*
> >>> -* RETURN VALUE
> >>> -*	This function returns 0 when rankning has succeded , otherwise 1.
> >>> -******/
> >>> -
> >>> -/****** OpenSM: UpDn/osm_subn_calc_up_down_min_hop_table
> >>> -* NAME
> >>> -*	osm_subn_calc_up_down_min_hop_table
> >>> -*
> >>> -* DESCRIPTION
> >>> -*	This function perform ranking and setting of all switches' min hop table
> >>> -*        by UP DOWN algorithm
> >>> -*
> >>> -* SYNOPSIS
> >>> -*/
> >>> -
> >>> -int
> >>> -osm_subn_calc_up_down_min_hop_table(
> >>> -  IN uint32_t num_guids,
> >>> -  IN uint64_t* guid_list,
> >>> -  IN updn_t* p_updn );
> >>> -
> >>> -/*
> >>> -* PARAMETERS
> >>> -*
> >>> -*	guid_list
> >>> -*		[in] Guid list from which to start ranking .
> >>> -*
> >>> -*	p_updn
> >>> -*		[in] Pointer to updn structure which includes state & lid2rank table
> >>> -* RETURN VALUE
> >>> -*	This function returns 0 when rankning has succeded , otherwise 1.
> >>> -******/
> >>> -
> >>> -/****** OpenSM: UpDn/osm_updn_find_root_nodes_by_min_hop
> >>> -* NAME
> >>> -*	osm_updn_find_root_nodes_by_min_hop
> >>> -*
> >>> -* DESCRIPTION
> >>> -*	This function perform auto identification of root nodes for UPDN ranking phase
> >>> -*
> >>> -* SYNOPSIS
> >>> -*/
> >>> -int
> >>> -osm_updn_find_root_nodes_by_min_hop( OUT updn_t *  p_updn );
> >>> -
> >>> -/*
> >>> -* PARAMETERS
> >>> -*	p_root_nodes_list
> >>> -*       
> >>> -*		[out] Pointer to the root nodes list found in the subnet
> >>> -*
> >>> -* RETURN VALUE
> >>> -*	This function returns 0 when auto identification had succeeded
> >>> -******/
> >>> -
> >>>  END_C_DECLS
> >>>  
> >>>  #endif /* _OSM_UCAST_UPDN_H_ */
> >>> diff --git a/osm/opensm/osm_ucast_updn.c b/osm/opensm/osm_ucast_updn.c
> >>> index 86ac3ad..0121e6e 100644
> >>> --- a/osm/opensm/osm_ucast_updn.c
> >>> +++ b/osm/opensm/osm_ucast_updn.c
> >>> @@ -55,8 +55,62 @@ #include <complib/cl_debug.h>
> >>>  #include <complib/cl_qmap.h>
> >>>  #include <opensm/osm_switch.h>
> >>>  #include <opensm/osm_opensm.h>
> >>> -#include <opensm/osm_ucast_updn.h>
> >>> -#include <stdlib.h>
> >>> +
> >>> +/* //////////////////////////// */
> >>> +/*  Local types */
> >>> +/* /////////////////////////// */
> >>> +
> >>> +/* direction */
> >>> +typedef enum _updn_switch_dir
> >>> +{
> >>> +    UP = 0,
> >>> +    DOWN
> >>> +} updn_switch_dir_t;
> >>> +
> >>> +/* This enum respresent available states in the UPDN algorithm */
> >>> +typedef enum _updn_state
> >>> +{
> >>> +    UPDN_INIT = 0,
> >>> +    UPDN_RANK,
> >>> +    UPDN_MIN_HOP_CALC,
> >>> +} updn_state_t;
> >>> +
> >>> +/* Rank value of this node */
> >>> +typedef struct _updn_rank
> >>> +{
> >>> +  cl_map_item_t map_item;
> >>> +  uint8_t rank;
> >>> +} updn_rank_t;
> >>> +
> >>> +/* Histogram element - the number of occurences of the same hop value */
> >>> +typedef struct _updn_hist
> >>> +{
> >>> +  cl_map_item_t map_item;
> >>> +  uint32_t bar_value;
> >>> +} updn_hist_t;
> >>> +
> >>> +typedef struct _updn_next_step
> >>> +{
> >>> +  updn_switch_dir_t state;
> >>> +  osm_switch_t *p_sw;
> >>> +} updn_next_step_t;
> >>> +
> >>> +/* guids list */
> >>> +typedef struct _updn_input
> >>> +{
> >>> +  uint32_t num_guids;
> >>> +  uint64_t * guid_list;
> >>> +} updn_input_t;
> >>> +
> >>> +/* updn structure */
> >>> +typedef struct _updn
> >>> +{
> >>> +  updn_state_t   state;
> >>> +  boolean_t      auto_detect_root_nodes;
> >>> +  cl_qmap_t      guid_rank_tbl;
> >>> +  updn_input_t   updn_ucast_reg_inputs;
> >>> +  cl_list_t *    p_root_nodes;
> >>> +} updn_t;
> >>>  
> >>>  
> >>>  /* ///////////////////////////////// */
> >>> @@ -65,6 +119,11 @@ #include <stdlib.h>
> >>>  /*  This var is predefined and initialized */
> >>>  extern osm_opensm_t osm;
> >>>  
> >>> +/* ///////////////////////////////// */
> >>> +/*  Statics                          */
> >>> +/* ///////////////////////////////// */
> >>> +static int osm_updn_find_root_nodes_by_min_hop(OUT updn_t *p_updn);
> >>> +
> >>>  /**********************************************************************
> >>>   **********************************************************************/
> >>>  /* This function returns direction based on rank and guid info of current &
> >>> @@ -471,7 +530,7 @@ __updn_bfs_by_node(
> >>>  
> >>>  /**********************************************************************
> >>>   **********************************************************************/
> >>> -void
> >>> +static void
> >>>  updn_destroy(
> >>>    IN updn_t* const p_updn )
> >>>  {
> >>> @@ -508,7 +567,7 @@ updn_destroy(
> >>>  
> >>>  /**********************************************************************
> >>>   **********************************************************************/
> >>> -updn_t*
> >>> +static updn_t*
> >>>  updn_construct(void)
> >>>  {
> >>>    updn_t* p_updn;
> >>> @@ -523,7 +582,7 @@ updn_construct(void)
> >>>  
> >>>  /**********************************************************************
> >>>   **********************************************************************/
> >>> -cl_status_t
> >>> +static cl_status_t
> >>>  updn_init(
> >>>    IN updn_t* const p_updn )
> >>>  {
> >>> @@ -635,7 +694,7 @@ updn_init(
> >>>   **********************************************************************/
> >>>  /* NOTE : PLS check if we need to decide that the first */
> >>>  /*        rank is a SWITCH for BFS purpose */
> >>> -int
> >>> +static int
> >>>  updn_subn_rank(
> >>>    IN uint64_t root_guid,
> >>>    IN uint8_t base_rank,
> >>> @@ -795,7 +854,7 @@ updn_subn_rank(
> >>>  
> >>>  /**********************************************************************
> >>>   **********************************************************************/
> >>> -int
> >>> +static int
> >>>  osm_subn_set_up_down_min_hop_table(
> >>>    IN updn_t* p_updn )
> >>>  {
> >>> @@ -880,7 +939,7 @@ osm_subn_set_up_down_min_hop_table(
> >>>  
> >>>  /**********************************************************************
> >>>   **********************************************************************/
> >>> -int
> >>> +static int
> >>>  osm_subn_calc_up_down_min_hop_table(
> >>>    IN uint32_t num_guids,
> >>>    IN uint64_t * guid_list,
> >>> @@ -935,7 +994,7 @@ osm_subn_calc_up_down_min_hop_table(
> >>>  /**********************************************************************
> >>>   **********************************************************************/
> >>>  /* UPDN callback function */
> >>> -int __osm_updn_call(
> >>> +static int __osm_updn_call(
> >>>    void *ctx )
> >>>  {
> >>>    OSM_LOG_ENTER(&(osm.log), __osm_updn_call);
> >>> @@ -969,7 +1028,7 @@ int __osm_updn_call(
> >>>  /**********************************************************************
> >>>   **********************************************************************/
> >>>  /* UPDN convert cl_list to guid array in updn struct */
> >>> -void __osm_updn_convert_list2array(
> >>> +static void __osm_updn_convert_list2array(
> >>>    IN updn_t * p_updn )
> >>>  {
> >>>    uint32_t i = 0, max_num = 0;
> >>> @@ -1008,7 +1067,7 @@ void __osm_updn_convert_list2array(
> >>>  /**********************************************************************
> >>>   **********************************************************************/
> >>>  /* Find Root nodes automatically by Min Hop Table info */
> >>> -int
> >>> +static int
> >>>  osm_updn_find_root_nodes_by_min_hop(
> >>>    OUT updn_t *  p_updn )
> >>>  {
> >>>       
> >
> > _______________________________________________
> > openib-general mailing list
> > openib-general at openib.org
> > http://openib.org/mailman/listinfo/openib-general
> >
> > To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
> >   
> 





More information about the general mailing list