[openib-general] [PATCH] opensm: strict osm_log arguments/format check
Yevgeny Kliteynik
kliteyn at dev.mellanox.co.il
Thu Nov 2 05:38:47 PST 2006
Hi Sasha.
Good catch with those missing arguments.
One question: in several places you used cl_hton64() to print guid.
Shouldn't there be cl_ntoh64() instead?
...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