Merge ~dirk.zimoch/epics-base:dynamicVxWorksVmeFunctionBinding into ~epics-core/epics-base/+git/epics-base:7.0

Proposed by Dirk Zimoch
Status: Merged
Approved by: Andrew Johnson
Approved revision: 251304e280c023ed72031d25981d6db4bd0b73d4
Merged at revision: 220a27bdec2492f9938ccf2a275ba7d04f06cf30
Proposed branch: ~dirk.zimoch/epics-base:dynamicVxWorksVmeFunctionBinding
Merge into: ~epics-core/epics-base/+git/epics-base:7.0
Diff against target: 110 lines (+31/-17)
1 file modified
modules/libcom/src/osi/os/vxWorks/devLibVMEOSD.c (+31/-17)
Reviewer Review Type Date Requested Status
Andrew Johnson Approve
Review via email: mp+367253@code.launchpad.net

Description of the change

Use dynamic binding for vxWorks BSP functions because some BSPs don't provide them.

To post a comment you must log in.
Revision history for this message
Andrew Johnson (anj) wrote :

Why not just replace the #if lines with if (...) { and leave the bodies of the functions untouched? That would make it easier for reviewers to confirm that you aren't changing the logic at all. As it is we have to understand both the original logic and what you've replaced it with and convince ourselves that their behavior will be the same, but using run-time detection instead of compile-time. Modern compilers should generate the same code for both versions so human readability is more important than conciseness.

I know that some people don't like having multiple return statements in a function; personally I disagree with that position, especially in simple cases like these where there are no resources that need releasing.

Revision history for this message
Dirk Zimoch (dirk.zimoch) wrote :

New version as suggested.

Revision history for this message
Andrew Johnson (anj) wrote :

Thanks.

review: Approve
Revision history for this message
mdavidsaver (mdavidsaver) wrote :
Revision history for this message
Dirk Zimoch (dirk.zimoch) wrote :

The problem here was that not every vxWorks system has a VME bus and thus loading EPICS base failed with unresolved symbols. This applies to each library/module that has vxWorks support for VME mixed with code for other useful stuff. At the moment if you want the other stuff you must have VME. So yes, that may apply to devLib2 as well.

However so far I only had one such system, that is an old CompactRIO fom NI. And on that system I do not use devLib2. So I have not noticed a problem.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/modules/libcom/src/osi/os/vxWorks/devLibVMEOSD.c b/modules/libcom/src/osi/os/vxWorks/devLibVMEOSD.c
index 82bec60..93d30fc 100644
--- a/modules/libcom/src/osi/os/vxWorks/devLibVMEOSD.c
+++ b/modules/libcom/src/osi/os/vxWorks/devLibVMEOSD.c
@@ -110,7 +110,21 @@ static long vxDevWriteProbe (unsigned wordSize, volatile void *ptr, const void *
110110
111static void *devA24Malloc(size_t size);111static void *devA24Malloc(size_t size);
112static void devA24Free(void *pBlock);112static void devA24Free(void *pBlock);
113static long devInit(void) { return 0;}113
114/* We don't know which functions are implemented in the BSP */
115static int (*sysIntEnableFunc)(int) = NULL;
116static int (*sysIntDisableFunc)(int) = NULL;
117static int (*sysIntEnablePICFunc)(int) = NULL;
118static int (*sysIntDisablePICFunc)(int) = NULL;
119
120static long devInit(void)
121{
122 sysIntEnableFunc = epicsFindSymbol ("sysIntEnable");
123 sysIntDisableFunc = epicsFindSymbol ("sysIntDisable");
124 sysIntDisablePICFunc = epicsFindSymbol ("sysIntDisablePIC");
125 sysIntEnablePICFunc = epicsFindSymbol ("sysIntEnablePIC");
126 return 0;
127}
114128
115static long vxDevConnectInterruptVME (129static long vxDevConnectInterruptVME (
116 unsigned vectorNumber,130 unsigned vectorNumber,
@@ -214,16 +228,16 @@ static long vxDevDisconnectInterruptVME (
214 */228 */
215static long vxDevEnableInterruptLevelVME (unsigned level)229static long vxDevEnableInterruptLevelVME (unsigned level)
216{230{
217# if CPU_FAMILY != I80X86 231 if (sysIntEnableFunc) {
218 int s;232 int s;
219 s = sysIntEnable (level);233 s = sysIntEnableFunc (level);
220 if (s!=OK) {234 if (s!=OK) {
221 return S_dev_intEnFail;235 return S_dev_intEnFail;
222 }236 }
223 return 0;237 return 0;
224# else238 } else {
225 return S_dev_intEnFail;239 return S_dev_intEnFail;
226# endif240 }
227}241}
228242
229/*243/*
@@ -231,16 +245,16 @@ static long vxDevEnableInterruptLevelVME (unsigned level)
231 */245 */
232long devEnableInterruptLevelISA (unsigned level)246long devEnableInterruptLevelISA (unsigned level)
233{247{
234# if CPU_FAMILY == I80X86248 if (sysIntEnablePICFunc) {
235 int s;249 int s;
236 s = sysIntEnablePIC (level);250 s = sysIntEnablePICFunc (level);
237 if (s!=OK) {251 if (s!=OK) {
238 return S_dev_intEnFail;252 return S_dev_intEnFail;
239 }253 }
240 return 0;254 return 0;
241# else255 } else {
242 return S_dev_intEnFail;256 return S_dev_intEnFail;
243# endif257 }
244}258}
245259
246/*260/*
@@ -248,15 +262,15 @@ long devEnableInterruptLevelISA (unsigned level)
248 */262 */
249long devDisableInterruptLevelISA (unsigned level)263long devDisableInterruptLevelISA (unsigned level)
250{264{
251# if CPU_FAMILY == I80X86265 if (sysIntDisablePICFunc) {
252 int s;266 int s;
253 s = sysIntDisablePIC (level);267 s = sysIntDisablePICFunc (level);
254 if (s!=OK) {268 if (s!=OK) {
255 return S_dev_intEnFail;269 return S_dev_intEnFail;
256 }270 }
257# else271 } else {
258 return S_dev_intEnFail;272 return S_dev_intEnFail;
259# endif273 }
260274
261 return 0;275 return 0;
262}276}
@@ -266,16 +280,16 @@ long devDisableInterruptLevelISA (unsigned level)
266 */280 */
267static long vxDevDisableInterruptLevelVME (unsigned level)281static long vxDevDisableInterruptLevelVME (unsigned level)
268{282{
269# if CPU_FAMILY != I80X86283 if (sysIntDisableFunc) {
270 int s;284 int s;
271 s = sysIntDisable (level);285 s = sysIntDisableFunc (level);
272 if (s!=OK) {286 if (s!=OK) {
273 return S_dev_intDissFail;287 return S_dev_intDissFail;
274 }288 }
275 return 0;289 return 0;
276# else290 } else {
277 return S_dev_intEnFail;291 return S_dev_intEnFail;
278# endif292 }
279}293}
280294
281/*295/*

Subscribers

People subscribed via source and target branches