[openib-general] [PATCH] opensm: updn: make local functions static + local types
Eitan Zahavi
eitan at mellanox.co.il
Mon Oct 23 03:07:55 PDT 2006
Hal Rosenstock wrote:
> 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.
>
Yes there is value in keeping a consistent code style across a project.
Every project I know has a style.
OpenSM style is there for many years. We can change it if we like but
let us do it consciously and on the entire tree.
>
>> 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 ?
>
YES - No coding style changes should be allowed on a per checkin basis.
Otherwise we turn the coding style into a mess.
> -- 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
>>>
>>>
>
>
> _______________________________________________
> 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