From 475c2b70493d6928a969975796a4eabf9fabb98a Mon Sep 17 00:00:00 2001
From: EJ Dohmen <ezekiel.dohmen@ligo.org>
Date: Thu, 27 Jan 2022 11:32:48 -0800
Subject: [PATCH] Cleaning up findSharedMemory usage, fixing error checks on
 the returned pointer, consolidating code for findSharedMemory() and
 findSharedMemorySize() so we don't have duplicate, removing unused sprintf in
 both functions

---
 src/drv/rfm.c          | 92 +++++++++++++++---------------------------
 src/fe/rcguser.c       |  1 -
 src/fe/rcguserCommon.c | 36 +++++++----------
 src/fe/rcguserIop.c    |  1 -
 4 files changed, 46 insertions(+), 84 deletions(-)

diff --git a/src/drv/rfm.c b/src/drv/rfm.c
index e1da4b783..8386e9518 100644
--- a/src/drv/rfm.c
+++ b/src/drv/rfm.c
@@ -18,8 +18,6 @@
 
 #include "mbuf/mbuf.h"
 
-/// Pointer to start of shared memory segment for a selected system.
-volatile unsigned char *addr = 0;
 
 ///  Search shared memory device file names in /rtl_mem_*
 ///	@param[in] *sys_name	Name of system, required to attach shared memory.
@@ -27,71 +25,45 @@ volatile unsigned char *addr = 0;
 volatile void *
 findSharedMemory(char *sys_name)
 {
-	char *s;
-        int fd;
-	char sys[128];
-	char fname[128];
-	strcpy(sys, sys_name);
-	for(s = sys; *s; s++) *s=tolower(*s);
+    int ss_mb = 64;
+    if (!strcmp(sys_name, "ipc")) ss_mb = 32;
+    if (!strcmp(sys_name, "shmipc")) ss_mb = 16;
 
-	sprintf(fname, "/rtl_mem_%s", sys);
-
-	int ss = 64*1024*1024;
-  	if (!strcmp(sys_name, "ipc")) ss = 32*1024*1024;
-  	if (!strcmp(sys_name, "shmipc")) ss = 16*1024*1024;
-
-       if ((fd = open ("/dev/mbuf", O_RDWR | O_SYNC)) < 0) {
-		fprintf(stderr, "Couldn't open /dev/mbuf read/write\n");
-                return 0;
-       }
-       struct mbuf_request_struct req;
-       req.size = ss;
-       strcpy(req.name, sys);
-       ioctl (fd, IOCTL_MBUF_ALLOCATE, &req);
-       ioctl (fd, IOCTL_MBUF_INFO, &req);
-	
-        addr = (volatile unsigned char *)mmap(0, ss, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
-        if (addr == MAP_FAILED) {
-                printf("return was %d\n",errno);
-                perror("mmap");
-                _exit(-1);
-        }
-	printf(" %s mmapped address is 0x%lx\n", sys,(long)addr);
-        return addr;
+    return findSharedMemorySize(sys_name, ss_mb);
 }
 
 volatile void *
-findSharedMemorySize(char *sys_name, int size)
+findSharedMemorySize(char *sys_name, int size_mb)
 {
-	char *s;
-        int fd;
-	char sys[128];
-	char fname[128];
-	strcpy(sys, sys_name);
-	for(s = sys; *s; s++) *s=tolower(*s);
+    volatile unsigned char *addr = 0;
+    char *s;
+    int fd;
+    char sys[128];
+    strcpy(sys, sys_name);
+    for(s = sys; *s; s++) *s=tolower(*s);
+
+
+    int size_bytes = size_mb*1024*1024;
+    printf("Making mbuff area %s with size %d\n", sys, size_bytes);
 
-	sprintf(fname, "/rtl_mem_%s", sys);
+    if ((fd = open ("/dev/mbuf", O_RDWR | O_SYNC)) < 0) {
+        fprintf(stderr, "Couldn't open /dev/mbuf read/write\n");
+        return 0;
+    }
 
-	int ss = size*1024*1024;
-	printf("Making mbuff area %s with size %d\n",sys,ss);
+    struct mbuf_request_struct req;
+    req.size = size_bytes;
+    strcpy(req.name, sys);
+    ioctl (fd, IOCTL_MBUF_ALLOCATE, &req);
+    ioctl (fd, IOCTL_MBUF_INFO, &req);
 
-       if ((fd = open ("/dev/mbuf", O_RDWR | O_SYNC)) < 0) {
-		fprintf(stderr, "Couldn't open /dev/mbuf read/write\n");
-                return 0;
-       }
-       struct mbuf_request_struct req;
-       req.size = ss;
-       strcpy(req.name, sys);
-       ioctl (fd, IOCTL_MBUF_ALLOCATE, &req);
-       ioctl (fd, IOCTL_MBUF_INFO, &req);
-	
-        addr = (volatile unsigned char *)mmap(0, ss, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
-        if (addr == MAP_FAILED) {
-                printf("return was %d\n",errno);
-                perror("mmap");
-                _exit(-1);
-        }
-	printf(" %s mmapped address is 0x%lx\n", sys,(long)addr);
-        return addr;
+    addr = (volatile unsigned char *)mmap(0, size_bytes, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+    if (addr == MAP_FAILED) {
+        printf("return was %d\n", errno);
+        perror("mmap");
+        _exit(-1);
+    }
+    printf(" %s mmapped address is 0x%lx\n", sys, (long)addr);
+    return addr;
 }
 
diff --git a/src/fe/rcguser.c b/src/fe/rcguser.c
index 1a72d9196..ecf8e3c6e 100644
--- a/src/fe/rcguser.c
+++ b/src/fe/rcguser.c
@@ -29,7 +29,6 @@
 // These externs and "16" need to go to a header file (mbuf.h)
 extern int   fe_start_app_user( );
 extern char  daqArea[ DAQ_DCU_SIZE ]; // Space allocation for daqLib buffers
-extern char* addr;
 
 /// Startup function for initialization of user space module.
 int
diff --git a/src/fe/rcguserCommon.c b/src/fe/rcguserCommon.c
index a342b74cc..0aeaefe6d 100644
--- a/src/fe/rcguserCommon.c
+++ b/src/fe/rcguserCommon.c
@@ -18,13 +18,11 @@ attach_shared_memory( char* sysname )
 {
 
     char         shm_name[ 64 ];
-    extern char* addr;
 
     // epics shm used to communicate with model's EPICS process
     sprintf( shm_name, "%s", sysname );
-    findSharedMemory( sysname );
-    _epics_shm = (char*)addr;
-    if ( _epics_shm < 0 )
+    _epics_shm = (char*) findSharedMemory( sysname );
+    if ( _epics_shm == NULL )
     {
         printf( "mbuf_allocate_area() failed; ret = %d\n", _epics_shm );
         return -1;
@@ -33,9 +31,8 @@ attach_shared_memory( char* sysname )
 
     // testpoint config shm used to control testpoints from awgtpman
     sprintf( shm_name, "%s%s", sysname, SHMEM_TESTPOINT_SUFFIX );
-    findSharedMemory( sysname );
-    _tp_shm = (volatile TESTPOINT_CFG *)addr;
-    if ( (uint64_t)_tp_shm < 0 )
+    _tp_shm = (volatile TESTPOINT_CFG *) findSharedMemory( sysname );
+    if ( _tp_shm == NULL )
     {
         printf( "mbuf_allocate_area(tp) failed; ret = %d\n", _tp_shm );
         return -1;
@@ -44,9 +41,8 @@ attach_shared_memory( char* sysname )
     
     // awg data shm used to stream data from awgtpman
     sprintf( shm_name, "%s%s", sysname, SHMEM_AWG_SUFFIX );
-    findSharedMemory( sysname );
-    _awg_shm = (volatile AWG_DATA *)addr;
-    if ( (uint64_t)_awg_shm < 0 )
+    _awg_shm = (volatile AWG_DATA *) findSharedMemory( sysname );
+    if ( _awg_shm == NULL )
     {
         printf( "mbuf_allocate_area(awg) failed; ret = %d\n", _awg_shm );
         return -1;
@@ -54,9 +50,8 @@ attach_shared_memory( char* sysname )
     printf( "AWGSM at 0x%lx\n", (long)_awg_shm );
     
     // ipc_shm used to communicate with IOP
-    findSharedMemory( "ipc" );
-    _ipc_shm = (char*)addr;
-    if ( _ipc_shm < 0 )
+    _ipc_shm = (char*)findSharedMemory( "ipc" );
+    if ( _ipc_shm == NULL )
     {
         printf( "mbuf_allocate_area(ipc) failed; ret = %d\n", _ipc_shm );
         return -1;
@@ -69,9 +64,8 @@ attach_shared_memory( char* sysname )
 
     // DAQ is via shared memory
     sprintf( shm_name, "%s_daq", sysname );
-    findSharedMemory( shm_name );
-    _daq_shm = (char*)addr;
-    if ( _daq_shm < 0 )
+    _daq_shm = (char*) findSharedMemory( shm_name );
+    if ( _daq_shm == NULL )
     {
         printf( "mbuf_allocate_area() failed; ret = %d\n", _daq_shm );
         return -1;
@@ -80,18 +74,16 @@ attach_shared_memory( char* sysname )
     daqPtr = (struct rmIpcStr*)_daq_shm;
 
     // shmipc is used to send SHMEM IPC data between processes on same computer
-    findSharedMemory( "shmipc" );
-    _shmipc_shm = (char*)addr;
-    if ( _shmipc_shm < 0 )
+    _shmipc_shm = (char*) findSharedMemory( "shmipc" );
+    if ( _shmipc_shm == NULL )
     {
         printf( "mbuf_allocate_area() failed; ret = %d\n", _shmipc_shm );
         return -1;
     }
 
     // Open new IO shared memory in support of no hardware I/O
-    findSharedMemory( "virtual_io_space" );
-    _io_shm = (char*)addr;
-    if ( _io_shm < 0 )
+    _io_shm = (char*) findSharedMemory( "virtual_io_space" );
+    if ( _io_shm == NULL )
     {
         printf( "mbuf_allocate_area() failed; ret = %d\n", _io_shm );
         return -1;
diff --git a/src/fe/rcguserIop.c b/src/fe/rcguserIop.c
index 5805bfade..84913bc10 100644
--- a/src/fe/rcguserIop.c
+++ b/src/fe/rcguserIop.c
@@ -22,7 +22,6 @@
 // These externs and "16" need to go to a header file (mbuf.h)
 extern int   fe_start_iop_user( );
 extern char  daqArea[ DAQ_DCU_SIZE ]; // Space allocation for daqLib buffers
-extern char* addr;
 extern int   cycleOffset;
 
 // Scan a double
-- 
GitLab