[openib-general] Re: [PATCH][DAPL] fix sparse warnings on kdapltest
James Lentini
jlentini at netapp.com
Wed May 4 09:27:12 PDT 2005
Thanks Tom. Committed in revision 2253.
An explanation of kdapltest is probably in order here. It is a test
tool that has grown organically over time. The entire test could use a
review for consistency and correctness. Here's a quick list of
things that could be done:
- review flow control, especially around disconnecting. There is a
known race condition at disconnect w/ certain non-standard command
lines (if the last operation is an RDMA, the target may exit before
the operation is launched).
- review naming conventions (e.g. use of "dapl_" prefix for files is
confusing since the test is not related to the dat-provider code
- separate user and kernel components more clearly, remove
kdapltest reliance on the user level DAT headers
james
On Mon, 2 May 2005, Tom Duffy wrote:
> Sparse fixes on kdapltest.
>
> Signed-off-by: Tom Duffy <tduffy at sun.com>
>
> Index: gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_server.c
> ===================================================================
> --- gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_server.c (revision 2247)
> +++ gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_server.c (working copy)
> @@ -597,7 +597,7 @@ DT_cs_Server (Params_t * params_ptr)
> DT_Mdep_Unlock (&ps_ptr->num_clients_lock);
>
> /* we passed the pt_ptr to the thread and must now 'forget' it */
> - pt_ptr = 0;
> + pt_ptr = NULL;
>
> ret = dat_ep_disconnect (ps_ptr->ep_handle, DAT_CLOSE_GRACEFUL_FLAG);
> if (ret != DAT_SUCCESS)
> @@ -796,7 +796,7 @@ server_exit:
> } /* end if ps_ptr */
>
> /* Clean up the server list */
> - pre_list = 0;
> + pre_list = NULL;
> temp_list = DT_started_server_list;
> while (temp_list)
> {
> Index: gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_test_data.c
> ===================================================================
> --- gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_test_data.c (revision 2247)
> +++ gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_test_data.c (working copy)
> @@ -37,7 +37,7 @@ Per_Test_Data_t *
> DT_Alloc_Per_Test_Data (DT_Tdep_Print_Head *phead)
> {
> Per_Test_Data_t *pt_ptr;
> - pt_ptr = 0;
> + pt_ptr = NULL;
>
> pt_ptr = DT_Mdep_Malloc (sizeof (Per_Test_Data_t));
> if (!pt_ptr)
> Index: gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_fft_util.c
> ===================================================================
> --- gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_fft_util.c (revision 2247)
> +++ gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_fft_util.c (working copy)
> @@ -74,10 +74,10 @@ int DT_ep_create (Params_t *params_ptr,
> {
> DAT_RETURN status;
> DT_Tdep_Print_Head *phead;
> - *conn_evd= 0;
> - *send_evd= 0;
> - *recv_evd= 0;
> - *cr_evd= 0;
> + *conn_evd = NULL;
> + *send_evd = NULL;
> + *recv_evd = NULL;
> + *cr_evd = NULL;
> phead = params_ptr->phead;
>
> status = DT_Tdep_evd_create (ia_handle, DEFAULT_QUEUE_LEN, DAT_HANDLE_NULL,
> @@ -125,18 +125,18 @@ int DT_ep_create (Params_t *params_ptr,
> /* function that initializes the connection struct */
> void DT_fft_init_conn_struct (FFT_Connection_t *conn)
> {
> - conn->ia_handle = 0;
> - conn->pz_handle = 0;
> - conn->psp_handle = 0;
> - conn->ep_handle = 0;
> - conn->cr_evd = 0;
> - conn->send_evd = 0;
> - conn->conn_evd = 0;
> - conn->recv_evd = 0;
> - conn->cr_handle = 0;
> - conn->remote_netaddr = 0;
> - conn->bpool = 0;
> - conn->pt_ptr = 0;
> + conn->ia_handle = NULL;
> + conn->pz_handle = NULL;
> + conn->psp_handle = NULL;
> + conn->ep_handle = NULL;
> + conn->cr_evd = NULL;
> + conn->send_evd = NULL;
> + conn->conn_evd = NULL;
> + conn->recv_evd = NULL;
> + conn->cr_handle = NULL;
> + conn->remote_netaddr = NULL;
> + conn->bpool = NULL;
> + conn->pt_ptr = NULL;
> conn->connected = false;
> }
>
> @@ -216,7 +216,7 @@ int DT_fft_destroy_conn_struct (Params_t
> }
> if (conn->bpool)
> {
> - DT_Bpool_Destroy (0, phead, conn->bpool);
> + DT_Bpool_Destroy (NULL, phead, conn->bpool);
> }
> if (conn->psp_handle)
> {
> @@ -296,8 +296,9 @@ void DT_fft_init_server (Params_t *param
> DT_assert_dat (phead, rc == DAT_SUCCESS);
>
> /* allocate memory for buffers */
> - conn->bpool = DT_BpoolAlloc (0, phead, conn->ia_handle, conn->pz_handle, NULL, NULL,
> - 8192, 2, DAT_OPTIMAL_ALIGNMENT, false, false);
> + conn->bpool = DT_BpoolAlloc (NULL, phead, conn->ia_handle, conn->pz_handle,
> + NULL, NULL, 8192, 2, DAT_OPTIMAL_ALIGNMENT,
> + false, false);
> DT_assert (phead, conn->bpool);
> cleanup:
> return;
> Index: gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_fft_mem.c
> ===================================================================
> --- gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_fft_mem.c (revision 2247)
> +++ gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_fft_mem.c (working copy)
> @@ -50,12 +50,12 @@ int DT_mem_generic (Params_t *params_ptr
> rc = 0;
> expect = 0;
> res = 1;
> - lmr_handle = 0;
> + lmr_handle = NULL;
> lmr_context = 0;
> reg_addr = 0;
> - alloc_ptr = 0;
> - ia_handle = 0;
> - pz_handle = 0;
> + alloc_ptr = NULL;
> + ia_handle = NULL;
> + pz_handle = NULL;
>
> DT_fft_init_client (params_ptr, cmd, &conn);
> DT_assert (phead, NULL != conn.ia_handle);
> @@ -63,7 +63,7 @@ int DT_mem_generic (Params_t *params_ptr
> if (flag == 2)
> {
> buffer_size = 0;
> - alloc_ptr = 0;
> + alloc_ptr = NULL;
> }
> else
> {
> @@ -115,7 +115,7 @@ int DT_mem_generic (Params_t *params_ptr
> rc = dat_lmr_free (lmr_handle);
> DT_assert_dat (phead, rc == DAT_SUCCESS);
> }
> - lmr_handle = 0;
> + lmr_handle = NULL;
>
> rc = dat_lmr_kcreate (conn.ia_handle,
> DAT_MEM_TYPE_VIRTUAL,
> Index: gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_fft_queryinfo.c
> ===================================================================
> --- gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_fft_queryinfo.c (revision 2247)
> +++ gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_fft_queryinfo.c (working copy)
> @@ -82,7 +82,7 @@ int DT_queryinfo_basic (Params_t *params
> buffer_size = BUFFSIZE * sizeof (unsigned char);
> phead = params_ptr->phead;
> reg_addr = 0;
> - alloc_ptr = 0;
> + alloc_ptr = NULL;
>
> ia_handle = NULL;
> pz_handle = NULL;
> Index: gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_server_info.c
> ===================================================================
> --- gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_server_info.c (revision 2247)
> +++ gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_server_info.c (working copy)
> @@ -27,7 +27,7 @@
>
> #include "dapl_proto.h"
>
> -Started_server_t *DT_started_server_list = 0;
> +Started_server_t *DT_started_server_list = NULL;
>
> void
> DT_Server_Info_Endian (Server_Info_t * server_info)
> Index: gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_transaction_test.c
> ===================================================================
> --- gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_transaction_test.c (revision 2247)
> +++ gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_transaction_test.c (working copy)
> @@ -558,7 +558,7 @@ DT_Transaction_Main (void *param)
>
> /* what, me query? just try to accept the connection */
> ret = dat_cr_accept (cr_handle,
> - 0, /* NULL for RSP */
> + NULL, /* NULL for RSP */
> 0, (DAT_PVOID)0 /* no private data */ );
> if (ret != DAT_SUCCESS)
> {
> Index: gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_memlist.c
> ===================================================================
> --- gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_memlist.c (revision 2247)
> +++ gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_memlist.c (working copy)
> @@ -31,7 +31,7 @@ void
> DT_MemListInit (Per_Test_Data_t * pt_ptr)
> {
> DT_Mdep_LockInit (&pt_ptr->MemListLock);
> - pt_ptr->MemListHead = 0;
> + pt_ptr->MemListHead = NULL;
> }
>
> void *
> @@ -42,13 +42,13 @@ DT_MemListAlloc (Per_Test_Data_t * pt_pt
> {
> void *buffptr;
> MemListEntry_t *entry_ptr;
> - buffptr = 0;
> - entry_ptr = 0;
> + buffptr = NULL;
> + entry_ptr = NULL;
>
> buffptr = DT_Mdep_Malloc (size);
> if (buffptr == 0)
> {
> - return 0;
> + return NULL;
> }
> if (pt_ptr == 0) /* not use mem_list */
> {
> @@ -58,7 +58,7 @@ DT_MemListAlloc (Per_Test_Data_t * pt_pt
> if (entry_ptr == 0)
> {
> DT_Mdep_Free (buffptr);
> - return 0;
> + return NULL;
> }
> strcpy (entry_ptr->filename, file);
> entry_ptr->MemType = t;
> @@ -82,7 +82,7 @@ DT_MemListFree (Per_Test_Data_t * pt_ptr
> return;
> }
> DT_Mdep_Lock (&pt_ptr->MemListLock);
> - pre = 0;
> + pre = NULL;
> cur = pt_ptr->MemListHead;
> while (cur)
> {
> @@ -91,12 +91,12 @@ DT_MemListFree (Per_Test_Data_t * pt_ptr
> if (!pre) /* first entry */
> {
> pt_ptr->MemListHead = cur->next;
> - cur->next = 0;
> + cur->next = NULL;
> }
> else
> {
> pre->next = cur->next;
> - cur->next = 0;
> + cur->next = NULL;
> }
> DT_Mdep_Free (ptr);
> DT_Mdep_Free (cur);
> Index: gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_bpool.c
> ===================================================================
> --- gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_bpool.c (revision 2247)
> +++ gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_bpool.c (working copy)
> @@ -37,7 +37,7 @@
> */
>
>
> -void DTs_CreateIovec (
> +static void DTs_CreateIovec (
> void **region_addr,
> u64 *buf_region,
> DAT_VLEN buffer_size,
> @@ -97,8 +97,8 @@ DT_BpoolAlloc (
> DAT_BOOLEAN enable_rdma_read)
> {
> unsigned char *module = "DT_BpoolAlloc";
> - unsigned char *alloc_ptr = 0;
> - Bpool *bpool_ptr = 0;
> + unsigned char *alloc_ptr = NULL;
> + Bpool *bpool_ptr = NULL;
> DAT_COUNT alloc_size, bpool_size;
> DAT_REGION_DESCRIPTION region;
> DAT_RETURN ret = DAT_SUCCESS;
> @@ -308,7 +308,7 @@ err:
> DT_MemListFree (pt_ptr, alloc_ptr);
> }
>
> - return ( 0 );
> + return NULL;
> }
>
> /*****************************************************************************/
> Index: gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_fft_endpoint.c
> ===================================================================
> --- gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_fft_endpoint.c (revision 2247)
> +++ gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_fft_endpoint.c (working copy)
> @@ -177,14 +177,14 @@ int DT_endpoint_case2 (Params_t *params_
> DT_Tdep_PT_Printf (phead, "\
> Description: try to destroy ep with descriptor still in working queue\n");
> res = 1;
> - bpool = 0;
> - pz_handle = 0;
> - ia_handle = 0;
> - ep_handle = 0;
> - send_evd = 0;
> - conn_evd = 0;
> - recv_evd = 0;
> - cr_evd = 0;
> + bpool = NULL;
> + pz_handle = NULL;
> + ia_handle = NULL;
> + ep_handle = NULL;
> + send_evd = NULL;
> + conn_evd = NULL;
> + recv_evd = NULL;
> + cr_evd = NULL;
> dev_name = cmd->device_name;
>
> rc = DT_ia_open (dev_name, &ia_handle);
> @@ -200,7 +200,7 @@ int DT_endpoint_case2 (Params_t *params_
> &recv_evd,
> &ep_handle);
> DT_assert_dat (phead, rc == DAT_SUCCESS);
> - bpool = DT_BpoolAlloc (0, phead, ia_handle, pz_handle, NULL, NULL, 4096, 1,
> + bpool = DT_BpoolAlloc (NULL, phead, ia_handle, pz_handle, NULL, NULL, 4096, 1,
> DAT_OPTIMAL_ALIGNMENT, false, false);
> DT_assert (phead, bpool != 0);
> DT_assert (phead, DT_post_recv_buffer (phead,
> @@ -227,7 +227,7 @@ int DT_endpoint_case2 (Params_t *params_
> cleanup:
> if (bpool)
> {
> - rc = DT_Bpool_Destroy (0, phead, bpool);
> + rc = DT_Bpool_Destroy (NULL, phead, bpool);
> DT_assert_clean (phead, rc != false);
> }
> if (pz_handle)
> Index: gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_fft_pz.c
> ===================================================================
> --- gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_fft_pz.c (revision 2247)
> +++ gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_fft_pz.c (working copy)
> @@ -47,8 +47,8 @@ int DT_pz_case0 ( Params_t *params_ptr,
> Description: Test if we can normally create pz and destroy it.\n");
>
> res=1;
> - ia_handle=0;
> - pz_handle =0;
> + ia_handle = NULL;
> + pz_handle = NULL;
> evd_handle = DAT_HANDLE_NULL;
> dev_name= cmd->device_name;
>
> @@ -88,13 +88,13 @@ int DT_pz_case1 (Params_t *params_ptr, F
> Description: try to destroy pz with vi still associated with it\n");
>
> res=1;
> - ia_handle=0;
> - pz_handle =0;
> - ep_handle=0;
> - conn_evd = 0;
> - send_evd = 0;
> - recv_evd = 0;
> - cr_evd = 0;
> + ia_handle = NULL;
> + pz_handle = NULL;
> + ep_handle = NULL;
> + conn_evd = NULL;
> + send_evd = NULL;
> + recv_evd = NULL;
> + cr_evd = NULL;
> dev_name= cmd->device_name;
>
> rc = DT_ia_open (dev_name, &ia_handle);
> @@ -174,9 +174,9 @@ int DT_pz_case2 (Params_t *params_ptr, F
> associated with it\n");
>
> res=1;
> - ia_handle=0;
> - pz_handle =0;
> - bpool=0;
> + ia_handle = NULL;
> + pz_handle = NULL;
> + bpool = NULL;
> dev_name= cmd->device_name;
>
> rc = DT_ia_open (dev_name, &ia_handle);
> @@ -186,7 +186,7 @@ int DT_pz_case2 (Params_t *params_ptr, F
> DT_assert_dat (phead, rc == DAT_SUCCESS);
>
> /* allocate and register bpool */
> - bpool = DT_BpoolAlloc (0, phead, ia_handle, pz_handle, NULL,
> + bpool = DT_BpoolAlloc (NULL, phead, ia_handle, pz_handle, NULL,
> NULL, BUFFSIZE, 1, DAT_OPTIMAL_ALIGNMENT,
> false, false);
> DT_assert (phead, bpool != 0);
> @@ -200,7 +200,7 @@ int DT_pz_case2 (Params_t *params_ptr, F
> cleanup:
>
> /* deregister and free bpool */
> - if (DT_Bpool_Destroy (0, phead, bpool)==false)
> + if (DT_Bpool_Destroy (NULL, phead, bpool)==false)
> {
> DT_Tdep_PT_Printf (phead, "Warning: Destroy bpool fails, reboot for cleanup\n");
> return 0;
> Index: gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_fft_hwconn.c
> ===================================================================
> --- gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_fft_hwconn.c (revision 2247)
> +++ gen2/users/jlentini/linux-kernel/test/dapltest/test/dapl_fft_hwconn.c (working copy)
> @@ -42,7 +42,7 @@ int DT_hwconn_case0 ( Params_t *params_p
> Description: Test if we can normally Open NIC and then close it\n");
>
> dev_name= cmd->device_name;
> - nic_handle=0;
> + nic_handle = NULL;
> evd_handle = DAT_HANDLE_NULL;
>
> rc=dat_ia_open ((const DAT_NAME_PTR)dev_name, 10, &evd_handle, &nic_handle);
> @@ -128,7 +128,7 @@ int DT_hwconn_case2 (Params_t *params_pt
>
> DT_Tdep_PT_Printf (phead, "\
> Description: Try to close nic with Nic handle is null (NIC not open)\n");
> - nic_handle=0;
> + nic_handle = NULL;
> rc=dat_ia_close (nic_handle, DAT_CLOSE_ABRUPT_FLAG);
> DT_assert_dat (phead, DAT_GET_TYPE (rc) ==DAT_INVALID_HANDLE);
>
> Index: gen2/users/jlentini/linux-kernel/test/dapltest/mdep/linux/dapl_mdep_kernel.c
> ===================================================================
> --- gen2/users/jlentini/linux-kernel/test/dapltest/mdep/linux/dapl_mdep_kernel.c (revision 2247)
> +++ gen2/users/jlentini/linux-kernel/test/dapltest/mdep/linux/dapl_mdep_kernel.c (working copy)
> @@ -241,7 +241,7 @@ DT_Mdep_Thread_Start_Routine (void *thre
> daemonize(__func__);
> #endif
> thread_ptr->function (thread_ptr->param);
> - return 0;
> + return NULL;
> }
>
> /*
> Index: gen2/users/jlentini/linux-kernel/test/dapltest/kdapl/kdapl_module.c
> ===================================================================
> --- gen2/users/jlentini/linux-kernel/test/dapltest/kdapl/kdapl_module.c (revision 2247)
> +++ gen2/users/jlentini/linux-kernel/test/dapltest/kdapl/kdapl_module.c (working copy)
> @@ -90,7 +90,7 @@ static int kdapltest_ioctl (struct inode
> break;
> }
> if (copy_from_user (local_params,
> - (void *)param,
> + (void __user *)param,
> sizeof (Params_t)))
> {
> DT_Mdep_Free (local_params);
> @@ -120,14 +120,14 @@ static int kdapltest_ioctl (struct inode
> case KDAPL_IOCTL_GET_PRINTF:
> {
> if (copy_from_user (&print_ioctl,
> - (void *)param,
> + (void __user *)param,
> sizeof (DT_get_printf_ioctl)))
> {
> rval = -EFAULT;
> break;
> }
> rval = KDT_Get_Print_Line (print_ioctl.cookie, &buffer[0]);
> - if (copy_to_user (print_ioctl.buffer,
> + if (copy_to_user ((void __user *)print_ioctl.buffer,
> &buffer[0],
> strlen (buffer)+1))
> {
> @@ -149,9 +149,9 @@ static int kdapltest_ioctl (struct inode
> /* fops */
> static struct file_operations kdapltest_fops =
> {
> - open: kdapltest_open,
> - release: kdapltest_release,
> - ioctl: kdapltest_ioctl,
> + .open = kdapltest_open,
> + .release = kdapltest_release,
> + .ioctl = kdapltest_ioctl,
> };
>
> static void
> @@ -169,7 +169,7 @@ DT_Tdep_Test_Thread (Params_t *user_para
> #endif
> }
>
> -__init
> +static __init
> int kdapltest_init (void)
> {
>
> @@ -183,7 +183,7 @@ int kdapltest_init (void)
> return (0);
> }
>
> -__exit
> +static __exit
> void kdapltest_exit (void)
> {
> unregister_chrdev (kdapltest_major, "kdapltest");
>
More information about the general
mailing list