[openib-general] [PATCH] opensm: strict osm_log arguments/format check
Sasha Khapyorsky
sashak at voltaire.com
Thu Nov 2 07:26:52 PST 2006
On 15:38 Thu 02 Nov , Yevgeny Kliteynik wrote:
> Hi Sasha.
>
> Good catch with those missing arguments.
It is compiler...
> One question: in several places you used cl_hton64() to print guid.
> Shouldn't there be cl_ntoh64() instead?
Right, it is mistake. Thanks for catching. Will resend.
Sasha
>
> ...and yes, I know that these two functions are actually the same macro :)
>
> Thanks
>
> -- Yevgeny
>
>
> Sasha Khapyorsky wrote:
> > This adds gcc attribute to osm_log() which causes the compiler to check
> > argument types against a format string. And also there are related fixes
> > in osm_log() usage in opensm and osmtest.
> >
> > Signed-off-by: Sasha Khapyorsky <sashak at voltaire.com>
> > ---
> > osm/include/opensm/osm_log.h | 8 +++++++-
> > osm/libvendor/osm_vendor_ibumad_sa.c | 2 +-
> > osm/opensm/main.c | 3 ++-
> > osm/opensm/osm_pkey_mgr.c | 1 +
> > osm/opensm/osm_port_info_rcv.c | 5 +++--
> > osm/opensm/osm_sa_informinfo.c | 4 ++--
> > osm/opensm/osm_sa_link_record.c | 8 ++++----
> > osm/opensm/osm_sa_mad_ctrl.c | 3 ++-
> > osm/opensm/osm_sa_response.c | 2 +-
> > osm/opensm/osm_sm_state_mgr.c | 3 ++-
> > osm/opensm/osm_sminfo_rcv.c | 9 +++++----
> > osm/opensm/osm_state_mgr.c | 8 ++++----
> > osm/osmtest/osmt_multicast.c | 12 +++++++-----
> > osm/osmtest/osmt_service.c | 6 +++---
> > osm/osmtest/osmtest.c | 8 ++++----
> > 15 files changed, 48 insertions(+), 34 deletions(-)
> >
> > diff --git a/osm/include/opensm/osm_log.h b/osm/include/opensm/osm_log.h
> > index 62f3a0c..2b24886 100644
> > --- a/osm/include/opensm/osm_log.h
> > +++ b/osm/include/opensm/osm_log.h
> > @@ -60,6 +60,12 @@
> > #include <iba/ib_types.h>
> > #include <stdio.h>
> >
> > +#ifdef __GNUC__
> > +#define STRICT_OSM_LOG_FORMAT __attribute__((format(printf, 3, 4)))
> > +#else
> > +#define STRICT_OSM_LOG_FORMAT
> > +#endif
> > +
> > #ifdef __cplusplus
> > # define BEGIN_C_DECLS extern "C" {
> > # define END_C_DECLS }
> > @@ -374,7 +380,7 @@ void
> > osm_log(
> > IN osm_log_t* const p_log,
> > IN const osm_log_level_t verbosity,
> > - IN const char *p_str, ... );
> > + IN const char *p_str, ... ) STRICT_OSM_LOG_FORMAT;
> >
> > void
> > osm_log_raw(
> > diff --git a/osm/libvendor/osm_vendor_ibumad_sa.c b/osm/libvendor/osm_vendor_ibumad_sa.c
> > index 7fd0655..7c4a2f7 100644
> > --- a/osm/libvendor/osm_vendor_ibumad_sa.c
> > +++ b/osm/libvendor/osm_vendor_ibumad_sa.c
> > @@ -853,7 +853,7 @@ osmv_query_sa(
> > if ( p_mpr_req->sgid_count + p_mpr_req->dgid_count > IB_MULTIPATH_MAX_GIDS )
> > {
> > osm_log( p_log, OSM_LOG_ERROR,
> > - "osmv_query_sa DBG:001 MULTIPATH_REC ",
> > + "osmv_query_sa DBG:001 MULTIPATH_REC "
> > "SGID count %d DGID count %d max count %d\n",
> > p_mpr_req->sgid_count, p_mpr_req->dgid_count,
> > IB_MULTIPATH_MAX_GIDS );
> > diff --git a/osm/opensm/main.c b/osm/opensm/main.c
> > index 729702a..752b546 100644
> > --- a/osm/opensm/main.c
> > +++ b/osm/opensm/main.c
> > @@ -460,7 +460,8 @@ parse_ignore_guids_file(IN char *guids_f
> > {
> > osm_log( &p_osm->log, OSM_LOG_ERROR,
> > "parse_ignore_guids_file: ERR 0601: "
> > - "Unable to open ignore guids file (%s)\n" );
> > + "Unable to open ignore guids file (%s)\n",
> > + guids_file_name );
> > status = IB_ERROR;
> > goto Exit;
> > }
> > diff --git a/osm/opensm/osm_pkey_mgr.c b/osm/opensm/osm_pkey_mgr.c
> > index f2cb221..735dc14 100644
> > --- a/osm/opensm/osm_pkey_mgr.c
> > +++ b/osm/opensm/osm_pkey_mgr.c
> > @@ -139,6 +139,7 @@ pkey_mgr_process_physical_port(
> > "pkey_mgr_process_physical_port: ERR 0503: "
> > "Failed to obtain P_Key 0x%04x block and index for node "
> > "0x%016" PRIx64 " port %u\n",
> > + ib_pkey_get_base( pkey ),
> > cl_ntoh64( osm_node_get_node_guid( p_node ) ),
> > osm_physp_get_port_num( p_physp ) );
> > return;
> > diff --git a/osm/opensm/osm_port_info_rcv.c b/osm/opensm/osm_port_info_rcv.c
> > index 95112dc..f6d3595 100644
> > --- a/osm/opensm/osm_port_info_rcv.c
> > +++ b/osm/opensm/osm_port_info_rcv.c
> > @@ -724,8 +724,9 @@ osm_pi_rcv_process(
> > {
> > osm_log( p_rcv->p_log, OSM_LOG_VERBOSE,
> > "osm_pi_rcv_process: "
> > - "Got light sweep response from remote port of parent node GUID = 0x%" PRIx64
> > - " port = %u, Commencing heavy sweep\n",
> > + "Got light sweep response from remote port of parent node "
> > + "GUID = 0x%" PRIx64 " port = 0x%016" PRIx64
> > + ", Commencing heavy sweep\n",
> > cl_ntoh64( node_guid ),
> > cl_ntoh64( port_guid ) );
> > osm_state_mgr_process( p_rcv->p_state_mgr,
> > diff --git a/osm/opensm/osm_sa_informinfo.c b/osm/opensm/osm_sa_informinfo.c
> > index 69dca1d..da96d35 100644
> > --- a/osm/opensm/osm_sa_informinfo.c
> > +++ b/osm/opensm/osm_sa_informinfo.c
> > @@ -163,8 +163,8 @@ __validate_ports_access_rights(
> > {
> > osm_log( p_rcv->p_log, OSM_LOG_ERROR,
> > "__validate_ports_access_rights: ERR 4301: "
> > - "Invalid port guid: 0x%016\n",
> > - portguid );
> > + "Invalid port guid: 0x%016" PRIx64 "\n",
> > + cl_hton64(portguid) );
> > valid = FALSE;
> > goto Exit;
> > }
> > diff --git a/osm/opensm/osm_sa_link_record.c b/osm/opensm/osm_sa_link_record.c
> > index 751023f..0ca9092 100644
> > --- a/osm/opensm/osm_sa_link_record.c
> > +++ b/osm/opensm/osm_sa_link_record.c
> > @@ -145,10 +145,10 @@ __osm_lr_rcv_build_physp_link(
> > osm_log( p_rcv->p_log, OSM_LOG_ERROR,
> > "__osm_lr_rcv_build_physp_link: ERR 1801: "
> > "Unable to acquire link record\n"
> > - "\t\t\t\tFrom port 0x%\n"
> > - "\t\t\t\tTo port 0x%\n"
> > - "\t\t\t\tFrom lid 0x%\n"
> > - "\t\t\t\tTo lid 0x%\n",
> > + "\t\t\t\tFrom port 0x%u\n"
> > + "\t\t\t\tTo port 0x%u\n"
> > + "\t\t\t\tFrom lid 0x%u\n"
> > + "\t\t\t\tTo lid 0x%u\n",
> > from_port, to_port,
> > cl_ntoh16(from_lid),
> > cl_ntoh16(to_lid) );
> > diff --git a/osm/opensm/osm_sa_mad_ctrl.c b/osm/opensm/osm_sa_mad_ctrl.c
> > index cd896b6..208f0d2 100644
> > --- a/osm/opensm/osm_sa_mad_ctrl.c
> > +++ b/osm/opensm/osm_sa_mad_ctrl.c
> > @@ -132,7 +132,8 @@ __osm_sa_mad_ctrl_process(
> > "__osm_sa_mad_ctrl_process: "
> > /* "Responding BUSY status since the dispatcher is already"*/
> > "Dropping MAD since the dispatcher is already"
> > - " overloaded with %u messages and queue time of:%u[msec]\n",
> > + " overloaded with %u messages and queue time of:"
> > + "%" PRIu64 "[msec]\n",
> > num_messages, last_dispatched_msg_queue_time_msec );
> >
> > /* send a busy response */
> > diff --git a/osm/opensm/osm_sa_response.c b/osm/opensm/osm_sa_response.c
> > index db36ea2..27f4e9d 100644
> > --- a/osm/opensm/osm_sa_response.c
> > +++ b/osm/opensm/osm_sa_response.c
> > @@ -117,7 +117,7 @@ osm_sa_send_error(
> > if (osm_exit_flag)
> > {
> > osm_log( p_resp->p_log, OSM_LOG_DEBUG,
> > - "osm_sa_send_error: ",
> > + "osm_sa_send_error: "
> > "Ignoring requested send after exit\n" );
> > goto Exit;
> > }
> > diff --git a/osm/opensm/osm_sm_state_mgr.c b/osm/opensm/osm_sm_state_mgr.c
> > index aadc43a..1ba5eda 100644
> > --- a/osm/opensm/osm_sm_state_mgr.c
> > +++ b/osm/opensm/osm_sm_state_mgr.c
> > @@ -247,7 +247,8 @@ __osm_sm_state_mgr_send_master_sm_info_r
> > {
> > osm_log( p_sm_mgr->p_log, OSM_LOG_ERROR,
> > "__osm_sm_state_mgr_send_master_sm_info_req: ERR 3203: "
> > - "No port object for GUID 0x%X\n", p_sm_mgr->master_guid );
> > + "No port object for GUID 0x%016" PRIx64 "\n",
> > + cl_hton64(p_sm_mgr->master_guid) );
> > goto Exit;
> > }
> >
> > diff --git a/osm/opensm/osm_sminfo_rcv.c b/osm/opensm/osm_sminfo_rcv.c
> > index 825b18b..7657e97 100644
> > --- a/osm/opensm/osm_sminfo_rcv.c
> > +++ b/osm/opensm/osm_sminfo_rcv.c
> > @@ -402,8 +402,8 @@ __osm_sminfo_rcv_process_set_request(
> > osm_log( p_rcv->p_log, OSM_LOG_VERBOSE,
> > "__osm_sminfo_rcv_process_set_request: "
> > "Received a STANDBY signal. Updating "
> > - "sm_state_mgr master_guid: 0x%X\n",
> > - p_rcv_smi->guid );
> > + "sm_state_mgr master_guid: 0x%016" PRIx64 "\n",
> > + cl_hton64(p_rcv_smi->guid) );
> > p_rcv->p_sm_state_mgr->master_guid = p_rcv_smi->guid;
> > }
> >
> > @@ -482,8 +482,9 @@ __osm_sminfo_rcv_process_get_sm(
> > /* we will poll it - as long as it lives - we should be in Standby. */
> > osm_log( p_rcv->p_log, OSM_LOG_VERBOSE,
> > "__osm_sminfo_rcv_process_get_sm: "
> > - "Found higher SM. Updating sm_state_mgr master_guid: 0x%X\n",
> > - p_sm->p_port->guid );
> > + "Found higher SM. Updating sm_state_mgr master_guid:"
> > + " 0x%016" PRIx64 "\n",
> > + cl_hton64(p_sm->p_port->guid) );
> > p_rcv->p_sm_state_mgr->master_guid = p_sm->p_port->guid;
> > }
> > break;
> > diff --git a/osm/opensm/osm_state_mgr.c b/osm/opensm/osm_state_mgr.c
> > index 28e0c4c..ad22d9e 100644
> > --- a/osm/opensm/osm_state_mgr.c
> > +++ b/osm/opensm/osm_state_mgr.c
> > @@ -481,7 +481,7 @@ __osm_state_mgr_signal_warning(
> > {
> > osm_log( p_mgr->p_log, OSM_LOG_VERBOSE,
> > "__osm_state_mgr_signal_warning: "
> > - "Invalid signal %s(%d) in state %s\n",
> > + "Invalid signal %s(%lu) in state %s\n",
> > osm_get_sm_signal_str( signal ),
> > signal, osm_get_sm_state_str( p_mgr->state ) );
> > }
> > @@ -500,7 +500,7 @@ __osm_state_mgr_signal_error(
> > else
> > osm_log( p_mgr->p_log, OSM_LOG_ERROR,
> > "__osm_state_mgr_signal_error: ERR 3303: "
> > - "Invalid signal %s(%d) in state %s\n",
> > + "Invalid signal %s(%lu) in state %s\n",
> > osm_get_sm_signal_str( signal ),
> > signal, osm_get_sm_state_str( p_mgr->state ) );
> > }
> > @@ -1480,8 +1480,8 @@ __osm_state_mgr_exists_other_master_sm(
> > {
> > osm_log( p_mgr->p_log, OSM_LOG_VERBOSE,
> > "__osm_state_mgr_exists_other_master_sm: "
> > - "Found remote master SM with guid:0x%X\n",
> > - p_sm->smi.guid );
> > + "Found remote master SM with guid:0x%016" PRIx64 "\n",
> > + cl_hton64(p_sm->smi.guid) );
> > p_sm_res = p_sm;
> > goto Exit;
> > }
> > diff --git a/osm/osmtest/osmt_multicast.c b/osm/osmtest/osmt_multicast.c
> > index 33a4f47..19f9d37 100644
> > --- a/osm/osmtest/osmt_multicast.c
> > +++ b/osm/osmtest/osmt_multicast.c
> > @@ -1885,8 +1885,9 @@ osmt_run_mcast_flow( IN osmtest_t * cons
> > {
> > osm_log( &p_osmt->log, OSM_LOG_ERROR,
> > "osmt_run_mcast_flow: ERR 0209: "
> > - "Validating MGID failed. MGID:0x%016" PRIx64 "\n",
> > - p_mc_res->mgid
> > + "Validating MGID failed. MGID:0x%016" PRIx64 ":%016" PRIx64 "\n",
> > + cl_ntoh64( p_mc_res->mgid.unicast.prefix ),
> > + cl_ntoh64( p_mc_res->mgid.unicast.interface_id )
> > );
> > status = IB_ERROR;
> > goto Exit;
> > @@ -2044,8 +2045,9 @@ osmt_run_mcast_flow( IN osmtest_t * cons
> > {
> > osm_log( &p_osmt->log, OSM_LOG_ERROR,
> > "osmt_run_mcast_flow: ERR 0212: "
> > - "Validating MGID failed. MGID:0x%016" PRIx64 "\n",
> > - p_mc_res->mgid
> > + "Validating MGID failed. MGID:0x%016" PRIx64 ":%016" PRIx64 "\n",
> > + cl_ntoh64( p_mc_res->mgid.unicast.prefix ),
> > + cl_ntoh64( p_mc_res->mgid.unicast.interface_id )
> > );
> > status = IB_ERROR;
> > goto Exit;
> > @@ -3345,7 +3347,7 @@ osmt_run_mcast_flow( IN osmtest_t * cons
> > /* Delete all MCG that are not of IPoIB */
> > osm_log( &p_osmt->log, OSM_LOG_INFO,
> > "osmt_run_mcast_flow : "
> > - "Cleanup all MCG that are not IPoIB...\n", cnt );
> > + "Cleanup all MCG that are not IPoIB...\n" );
> >
> > p_mgrp_mlid_tbl = &p_osmt->exp_subn.mgrp_mlid_tbl;
> > p_mgrp = (osmtest_mgrp_t*)cl_qmap_head( p_mgrp_mlid_tbl );
> > diff --git a/osm/osmtest/osmt_service.c b/osm/osmtest/osmt_service.c
> > index ec9a39e..ab95fec 100644
> > --- a/osm/osmtest/osmt_service.c
> > +++ b/osm/osmtest/osmt_service.c
> > @@ -1559,7 +1559,7 @@ osmt_run_service_records_flow( IN osmtes
> > {
> > osm_log( &p_osmt->log, OSM_LOG_ERROR,
> > "osmt_run_service_records_flow: ERR 4A20: "
> > - "Found service: id: 0x%016 " PRIx64
> > + "Found service: id: 0x%016" PRIx64 " "
> > "that is invalid\n",
> > id[7] );
> > status = IB_ERROR;
> > @@ -1573,7 +1573,7 @@ osmt_run_service_records_flow( IN osmtes
> > {
> > osm_log( &p_osmt->log, OSM_LOG_ERROR,
> > "osmt_run_service_records_flow: ERR 4A21: "
> > - "Fail to find service: id: 0x%016 " PRIx64
> > + "Fail to find service: id: 0x%016" PRIx64 " "
> > "name: %s\n",
> > id[0],
> > (char*)service_name[0] );
> > @@ -1588,7 +1588,7 @@ osmt_run_service_records_flow( IN osmtes
> > {
> > osm_log( &p_osmt->log, OSM_LOG_ERROR,
> > "osmt_run_service_records_flow: ERR 4A22: "
> > - "Fail to find service: id: 0x%016 " PRIx64
> > + "Fail to find service: id: 0x%016" PRIx64 " "
> > "name: %s\n",
> > id[5],
> > (char*)service_name[6] );
> > diff --git a/osm/osmtest/osmtest.c b/osm/osmtest/osmtest.c
> > index 92a4190..a35e0c5 100644
> > --- a/osm/osmtest/osmtest.c
> > +++ b/osm/osmtest/osmtest.c
> > @@ -2787,7 +2787,8 @@ osmtest_create_inventory_file( IN osmtes
> > {
> > osm_log( &p_osmt->log, OSM_LOG_ERROR,
> > "osmtest_create_inventory_file: ERR 0079: "
> > - "Unable to open inventory file (%s)\n" );
> > + "Unable to open inventory file (%s)\n",
> > + p_osmt->opt.file_name );
> > status = IB_ERROR;
> > goto Exit;
> > }
> > @@ -3356,7 +3357,7 @@ osmtest_validate_path_data( IN osmtest_t
> > osm_log( &p_osmt->log, OSM_LOG_ERROR,
> > "osmtest_validate_path_data: ERR 0012: "
> > "PKEY mismatch on path SLID 0x%X to DLID 0x%X\n"
> > - "\t\t\t\tExpected 0x%X, received 0x%X\n",
> > + "\t\t\t\tExpected 0x%" PRIx64 ", received 0x%" PRIx64 "\n",
> > cl_ntoh16( p_path->rec.slid ),
> > cl_ntoh16( p_path->rec.dlid ),
> > cl_ntoh64( p_path->rec.pkey ), cl_ntoh64( p_rec->pkey ) );
> > @@ -7165,8 +7166,7 @@ osmtest_bind( IN osmtest_t * p_osmt,
> > {
> > osm_log( &p_osmt->log, OSM_LOG_ERROR,
> > "osmtest_bind: ERR 0135: "
> > - "No local ports. Unable to proceed\n",
> > - ib_get_err_str( status ) );
> > + "No local ports. Unable to proceed\n" );
> > goto Exit;
> > }
> > guid = attr_array[port_index].port_guid;
More information about the general
mailing list