[openib-general] Re: [PATCH] opensm: remove osm_pkey_mgr.h
Sasha Khapyorsky
sashak at voltaire.com
Mon May 22 02:14:09 PDT 2006
Hi Eitan,
On 09:27 Mon 22 May , Eitan Zahavi wrote:
>
> Every OpenSM manager has an H file.
Even if it is not necessary? Why?
> Instead of trying to "save" lines of code - please focus on improving
> code readability and structure.
This is exactly my point - to improve code readability and structure
(and obviously this is not last change needed in this area). And I
cannot see how huge amount of duplicated code and useless structures
may help there. Instead I can see how it hurts.
> NOTE: this is not kernel code. The tradeoff for user land code is
> different.
Yes, this is not kernel code, but I don't see your point. (don't think
that you are referring kernel sources as example of unreadable and bad
structured code :))
Probably you see this patch as "extremal optimization"? I see it as
cleanup and structure improvement.
Sasha
> If you save us one header file - but break the structure of the code you
> make more damage than good.
>
> Eitan Zahavi
> Senior Engineering Director, Software Architect
> Mellanox Technologies LTD
> Tel:+972-4-9097208
> Fax:+972-4-9593245
> P.O. Box 586 Yokneam 20692 ISRAEL
>
>
> > -----Original Message-----
> > From: Sasha Khapyorsky [mailto:sashak at voltaire.com]
> > Sent: Monday, May 22, 2006 1:16 AM
> > To: Hal Rosenstock
> > Cc: openib-general at openib.org; Eitan Zahavi; Yael Kalka; Ofer Gigi
> > Subject: [PATCH] opensm: remove osm_pkey_mgr.h
> >
> >
> > Since we expect that osm_pkey_mgr_process() will be called only from
> > osm_state_mgr_process() this patch replaces osm_pkey_mgr.h header file
> > by local prototype.
> >
> > Signed-off-by: Sasha Khapyorsky <sashak at voltaire.com>
> > ---
> >
> > osm/include/Makefile.am | 1
> > osm/include/opensm/osm_pkey_mgr.h | 92
> -------------------------------------
> > osm/opensm/osm_pkey_mgr.c | 1
> > osm/opensm/osm_state_mgr.c | 3 +
> > 4 files changed, 2 insertions(+), 95 deletions(-)
> >
> > diff --git a/osm/include/Makefile.am b/osm/include/Makefile.am
> > index b23b1de..2bee762 100644
> > --- a/osm/include/Makefile.am
> > +++ b/osm/include/Makefile.am
> > @@ -96,7 +96,6 @@ EXTRA_DIST = \
> > $(srcdir)/opensm/st.h \
> > $(srcdir)/opensm/osm_mcast_tbl.h \
> > $(srcdir)/opensm/osm_pkey.h \
> > - $(srcdir)/opensm/osm_pkey_mgr.h \
> > $(srcdir)/opensm/osm_sa_mad_ctrl.h \
> > $(srcdir)/opensm/osm_req_ctrl.h \
> > $(srcdir)/opensm/osm_sw_info_rcv.h \
> > diff --git a/osm/include/opensm/osm_pkey_mgr.h
> > b/osm/include/opensm/osm_pkey_mgr.h
> > deleted file mode 100644
> > index cb0075d..0000000
> > --- a/osm/include/opensm/osm_pkey_mgr.h
> > +++ /dev/null
> > @@ -1,92 +0,0 @@
> > -/*
> > - * Copyright (c) 2006 Voltaire, Inc. All rights reserved.
> > - * Copyright (c) 2002-2005 Mellanox Technologies LTD. All rights
> reserved.
> > - *
> > - * 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.
> > - *
> > - * $Id$
> > - */
> > -
> > -
> > -/*
> > - * Abstract:
> > - * Prototype for osm_pkey_mgr_process() function
> > - * This is part of the OpenSM family of objects.
> > - *
> > - * Environment:
> > - * Linux User Mode
> > - *
> > - * $Revision: 1.4 $
> > - */
> > -
> > -
> > -#ifndef _OSM_PKEY_MGR_H_
> > -#define _OSM_PKEY_MGR_H_
> > -
> > -#include <opensm/osm_base.h>
> > -#include <opensm/osm_opensm.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
> > -
> > -/****f* OpenSM: P_Key Manager/osm_pkey_mgr_process
> > -* NAME
> > -* osm_pkey_mgr_process
> > -*
> > -* DESCRIPTION
> > -* This function enforces the pkey rules on the SM DB.
> > -*
> > -* SYNOPSIS
> > -*/
> > -osm_signal_t
> > -osm_pkey_mgr_process(
> > - IN osm_opensm_t *p_osm );
> > -/*
> > -* PARAMETERS
> > -* p_osm
> > -* [in] Pointer to an osm_opensm_t object.
> > -*
> > -* RETURN VALUES
> > -* None
> > -*
> > -* NOTES
> > -*
> > -* SEE ALSO
> > -*********/
> > -
> > -END_C_DECLS
> > -
> > -#endif /* _OSM_PKEY_MGR_H_ */
> > diff --git a/osm/opensm/osm_pkey_mgr.c b/osm/opensm/osm_pkey_mgr.c
> > index e08b7cc..91c1a95 100644
> > --- a/osm/opensm/osm_pkey_mgr.c
> > +++ b/osm/opensm/osm_pkey_mgr.c
> > @@ -56,7 +56,6 @@ #include <complib/cl_qmap.h>
> > #include <complib/cl_debug.h>
> > #include <opensm/osm_node.h>
> > #include <opensm/osm_switch.h>
> > -#include <opensm/osm_pkey_mgr.h>
> > #include <opensm/osm_partition.h>
> > #include <opensm/osm_opensm.h>
> >
> > diff --git a/osm/opensm/osm_state_mgr.c b/osm/opensm/osm_state_mgr.c
> > index 42fd5e8..724b2b7 100644
> > --- a/osm/opensm/osm_state_mgr.c
> > +++ b/osm/opensm/osm_state_mgr.c
> > @@ -66,14 +66,15 @@ #include <opensm/osm_helper.h>
> > #include <opensm/osm_msgdef.h>
> > #include <opensm/osm_node.h>
> > #include <opensm/osm_port.h>
> > -#include <opensm/osm_pkey_mgr.h>
> > #include <vendor/osm_vendor_api.h>
> > #include <opensm/osm_sm_state_mgr.h>
> > #include <opensm/osm_opensm.h>
> >
> >
> /**********************************************************************
> > + * Prototypes for manager processors used locally
> >
> **********************************************************************/
> > osm_signal_t osm_qos_setup(IN osm_opensm_t * p_osm);
> > +osm_signal_t osm_pkey_mgr_process(IN osm_opensm_t * p_osm);
> >
> >
> /**********************************************************************
> >
> **********************************************************************/
More information about the general
mailing list