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

Sasha Khapyorsky sashak at voltaire.com
Mon Oct 23 04:16:52 PDT 2006


Hi Eitan,

On 09:20 Mon 23 Oct     , 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.

Maybe I was unclear - this patch does nothing with function or types
renaming. This is "make static" patch.

> Meanwhile lets keep the style as it is.

Nobody cared to unify up/down code before, and it clearly was not goal
of this patch too.

In general I think it is not bad thing to define recommended coding
style, but this is different issue (looks we will need to write
something about it too).

Sasha

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