[ofa-general] Re: [PATCH 5/5] Move extern declarations for node description processing to a new header file.
Ira Weiny
weiny2 at llnl.gov
Tue Mar 25 11:40:56 PDT 2008
On Mon, 24 Mar 2008 02:14:13 +0000
Sasha Khapyorsky <sashak at voltaire.com> wrote:
> Hi Ira,
>
> On 15:51 Fri 21 Mar , Ira Weiny wrote:
> > From 93479e6cac589167f79f09d42900994e947c90f4 Mon Sep 17 00:00:00 2001
> > From: Ira K. Weiny <weiny2 at llnl.gov>
> > Date: Fri, 21 Mar 2008 15:23:16 -0700
> > Subject: [PATCH] Move extern declarations for node description processing to a new header file.
> >
> >
> > Signed-off-by: Ira K. Weiny <weiny2 at llnl.gov>
> > ---
> > opensm/include/opensm/osm_node_desc.h | 54 +++++++++++++++++++++++++++++++++
> > opensm/opensm/osm_node_desc_rcv.c | 27 ++++++++++++++++
> > opensm/opensm/osm_node_info_rcv.c | 26 ----------------
> > opensm/opensm/osm_sm.c | 2 +-
> > opensm/opensm/osm_trap_rcv.c | 3 +-
> > 5 files changed, 83 insertions(+), 29 deletions(-)
> > create mode 100644 opensm/include/opensm/osm_node_desc.h
> >
> > diff --git a/opensm/include/opensm/osm_node_desc.h b/opensm/include/opensm/osm_node_desc.h
> > new file mode 100644
> > index 0000000..d86f6ba
> > --- /dev/null
> > +++ b/opensm/include/opensm/osm_node_desc.h
>
> When new file is added it should be also added to EXTRA_DIST list in
> include/Makefile.am so 'make dist' will work.
My bad, I should have checked the dist and rpm builds... Sorry.
>
> > @@ -0,0 +1,54 @@
> > +/*
> > + * Copyright (c) 2008 Lawrence Livermore National Security
> > + *
> > + * This software is available to you under a choice of one of two
> > + * licenses. You may choose to be licensed under the terms of the GNU
> > + * General Public License (GPL) Version 2, available from the file
> > + * COPYING in the main directory of this source tree, or the
> > + * OpenIB.org BSD license below:
> > + *
> > + * Redistribution and use in source and binary forms, with or
> > + * without modification, are permitted provided that the following
> > + * conditions are met:
> > + *
> > + * - Redistributions of source code must retain the above
> > + * copyright notice, this list of conditions and the following
> > + * disclaimer.
> > + *
> > + * - Redistributions in binary form must reproduce the above
> > + * copyright notice, this list of conditions and the following
> > + * disclaimer in the documentation and/or other materials
> > + * provided with the distribution.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> > + * SOFTWARE.
> > + *
> > + */
> > +
> > +#ifndef _OSM_NODE_DESC_H_
> > +#define _OSM_NODE_DESC_H_
> > +
> > +#include <opensm/osm_sm.h>
> > +#include <opensm/osm_port.h>
> > +
> > +#ifdef __cplusplus
> > +# define BEGIN_C_DECLS extern "C" {
> > +# define END_C_DECLS }
> > +#else /* !__cplusplus */
> > +# define BEGIN_C_DECLS
> > +# define END_C_DECLS
> > +#endif /* __cplusplus */
> > +
> > +BEGIN_C_DECLS
> > +
> > +void osm_nd_rcv_process(void *context, void *data);
> > +void osm_req_get_node_desc(osm_sm_t * sm, osm_physp_t *p_physp);
> > +
> > +END_C_DECLS
> > +#endif /* _OSM_NODE_DESC_H_ */
> > diff --git a/opensm/opensm/osm_node_desc_rcv.c b/opensm/opensm/osm_node_desc_rcv.c
> > index 4a22aab..a7266d5 100644
> > --- a/opensm/opensm/osm_node_desc_rcv.c
> > +++ b/opensm/opensm/osm_node_desc_rcv.c
> > @@ -134,3 +134,30 @@ void osm_nd_rcv_process(IN void *context, IN void *data)
> > CL_PLOCK_RELEASE(sm->p_lock);
> > OSM_LOG_EXIT(sm->p_log);
> > }
> > +
> > +/**********************************************************************
> > + The plock must be held before calling this function.
> > +**********************************************************************/
> > +void
> > +osm_req_get_node_desc(IN osm_sm_t * sm,
> > + osm_physp_t *p_physp)
> > +{
> > + ib_api_status_t status = IB_SUCCESS;
> > + osm_madw_context_t context;
> > +
> > + OSM_LOG_ENTER(sm->p_log);
> > +
> > + context.nd_context.node_guid =
> > + osm_node_get_node_guid(osm_physp_get_node_ptr(p_physp));
> > +
> > + status = osm_req_get(sm, osm_physp_get_dr_path_ptr(p_physp),
> > + IB_MAD_ATTR_NODE_DESC,
> > + 0, CL_DISP_MSGID_NONE, &context);
> > + if (status != IB_SUCCESS)
> > + OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 0D03: "
> > + "Failure initiating NodeDescription request (%s)\n",
> > + ib_get_err_str(status));
> > +
> > + OSM_LOG_EXIT(sm->p_log);
> > +}
> > +
> > diff --git a/opensm/opensm/osm_node_info_rcv.c b/opensm/opensm/osm_node_info_rcv.c
> > index 6818e05..9a3eb7b 100644
> > --- a/opensm/opensm/osm_node_info_rcv.c
> > +++ b/opensm/opensm/osm_node_info_rcv.c
> > @@ -306,32 +306,6 @@ __osm_ni_rcv_process_new_node(IN osm_sm_t * sm,
> > /**********************************************************************
> > The plock must be held before calling this function.
> > **********************************************************************/
> > -void
> > -osm_req_get_node_desc(IN osm_sm_t * sm,
> > - osm_physp_t *p_physp)
> > -{
> > - ib_api_status_t status = IB_SUCCESS;
> > - osm_madw_context_t context;
> > -
> > - OSM_LOG_ENTER(sm->p_log);
> > -
> > - context.nd_context.node_guid =
> > - osm_node_get_node_guid(osm_physp_get_node_ptr(p_physp));
> > -
> > - status = osm_req_get(sm, osm_physp_get_dr_path_ptr(p_physp),
> > - IB_MAD_ATTR_NODE_DESC,
> > - 0, CL_DISP_MSGID_NONE, &context);
> > - if (status != IB_SUCCESS)
> > - OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 0D03: "
> > - "Failure initiating NodeDescription request (%s)\n",
> > - ib_get_err_str(status));
> > -
> > - OSM_LOG_EXIT(sm->p_log);
> > -}
> > -
> > -/**********************************************************************
> > - The plock must be held before calling this function.
> > -**********************************************************************/
>
> Do we need this move?
I think this is appropriate to move all the node description functionality to
one file. I think it is awkward to have a file called osm_node_desc_rcv.c and
then have a function called osm_req_get_node_desc in a file with the name
*node_info*. Obviously grep finds all, but I thought this would be better
for the organization of the code.
>
> > static void
> > __osm_ni_rcv_get_node_desc(IN osm_sm_t * sm,
> > IN osm_node_t * const p_node,
> > diff --git a/opensm/opensm/osm_sm.c b/opensm/opensm/osm_sm.c
> > index 32525ba..69aafb4 100644
> > --- a/opensm/opensm/osm_sm.c
> > +++ b/opensm/opensm/osm_sm.c
> > @@ -64,12 +64,12 @@
> > #include <opensm/osm_mcm_info.h>
> > #include <opensm/osm_perfmgr.h>
> > #include <opensm/osm_opensm.h>
> > +#include <opensm/osm_node_desc.h>
> >
> > #define OSM_SM_INITIAL_TID_VALUE 0x1233
> >
> > extern void osm_lft_rcv_process(IN void *context, IN void *data);
> > extern void osm_mft_rcv_process(IN void *context, IN void *data);
> > -extern void osm_nd_rcv_process(IN void *context, IN void *data);
> > extern void osm_ni_rcv_process(IN void *context, IN void *data);
> > extern void osm_pkey_rcv_process(IN void *context, IN void *data);
> > extern void osm_pi_rcv_process(IN void *context, IN void *data);
>
> Isn't it would be better to put all those declarations to n one place
> (let's say osm_sm.h) instead of creating new - one function prototype
> header file?
I'm not sure... But there is already an osm_sm.h file and it has the functions
for the osm_sm_t object. For all these functions I think I would call the file
something like osm_rcv_callback.h. Since these all define callbacks used when
MADs of those types are received. However, the problem I was trying to solve
is where to put the declaration of osm_req_get_node_desc. At the time it
seemed best to put it with other node_desc type functions but perhaps it would
be best to create another c file with generic MAD sends in it? I am not sure
what it would be called at this time.
Ira
More information about the general
mailing list