Merge ~bfrk/epics-base:remove-dbfl_type_rec into ~epics-core/epics-base/+git/epics-base:7.0
- Git
- lp:~bfrk/epics-base
- remove-dbfl_type_rec
- Merge into 7.0
Status: | Merged |
---|---|
Merged at revision: | 504191441d8fecf44c071d4983f2c645253d6321 |
Proposed branch: | ~bfrk/epics-base:remove-dbfl_type_rec |
Merge into: | ~epics-core/epics-base/+git/epics-base:7.0 |
Diff against target: |
913 lines (+243/-289) 12 files modified
modules/database/src/ioc/db/dbAccess.c (+24/-25) modules/database/src/ioc/db/dbChannel.c (+17/-55) modules/database/src/ioc/db/dbChannel.h (+4/-3) modules/database/src/ioc/db/dbEvent.c (+36/-33) modules/database/src/ioc/db/dbExtractArray.c (+19/-49) modules/database/src/ioc/db/dbExtractArray.h (+30/-5) modules/database/src/ioc/db/db_field_log.h (+35/-27) modules/database/src/std/filters/arr.c (+39/-73) modules/database/src/std/filters/ts.c (+32/-9) modules/database/test/ioc/db/dbChArrTest.cpp (+1/-1) modules/database/test/std/filters/arrTest.cpp (+3/-3) modules/database/test/std/filters/dbndTest.c (+3/-6) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Ben Franksen (community) | Approve | ||
EPICS Core Developers | Pending | ||
Review via email: mp+396197@code.launchpad.net |
This proposal supersedes a proposal from 2020-03-31.
Commit message
Description of the change
This is just refactors, no new features yet.
Ben Franksen (bfrk) wrote : Posted in a previous version of this proposal | # |
Ben Franksen (bfrk) wrote : Posted in a previous version of this proposal | # |
I rebased and resubmitted to get rid of the reformat commit. Will create a different merge request for that one.
Dirk Zimoch (dirk.zimoch) wrote : Posted in a previous version of this proposal | # |
Apropos reformat commit: Should we do a general reformat of EPICS base? For example get rid of all the tabs.
Ben Franksen (bfrk) wrote : Posted in a previous version of this proposal | # |
> Apropos reformat commit: Should we do a general reformat of EPICS base? For
> example get rid of all the tabs.
I suggest to move this discussion to https:/
Ben Franksen (bfrk) wrote : Posted in a previous version of this proposal | # |
I have renamed my branch back to write-filters and removed the latest commit.
Ben Franksen (bfrk) wrote : Posted in a previous version of this proposal | # |
Okay, fresh and clean. Fixed the problem Ralph pointed out (in https:/
Andrew Johnson (anj) wrote : Posted in a previous version of this proposal | # |
I haven't tried building or using this, but overall it looks reasonable. Unfortunately I don't think we can afford to change the prototype of dbGetField() like this because it is a published API that other modules call, but AFAIK they only ever pass NULL for the final argument – see my diff comment below for suggestions.
Ben Franksen (bfrk) wrote : Posted in a previous version of this proposal | # |
Am 31.03.20 um 22:17 schrieb Andrew Johnson:
> I haven't tried building or using this, but overall it looks reasonable. Unfortunately I don't think we can afford to change the prototype of dbGetField() like this because it is a published API that other modules call, but AFAIK they only ever pass NULL for the final argument – see my diff comment below for suggestions.
I offered this particular patch (commit 7287a1f) as kind of "for
discussion", as I wasn't sure how much external code still uses this
API. I only saw that the record types and device supports in base no
longer call dbGetField directly. BTW, the void *pflin member must have
been added at some point in time (it wasn't there in 3.14) and that must
have taken its toll on external modules, too. But that was a "major"
version jump so perhaps the rules allowed that.
Anyway, I can easily remove this particular patch. It is pretty much
independent from anything else I did or plan to do.
On another note, my current (unfinished) attempts at adding put filters
also include the addition of a new parameter to dbPut and dbPutField.
This is a similarly breaking change, and I guess I should do something
about that before I propose that code for merging, right?
>> diff --git a/modules/
>> index 1534517..a801142 100644
>> --- a/modules/
>> +++ b/modules/
>> @@ -81,6 +92,8 @@ struct dbfl_val {
>> * db_delete_
>> * field log to another type, or to reference different data,
>> * must explicitly call the dtor function.
>> + * If the dtor is NULL, then this means the array data is still owned
>> + * by a record.
>
> Would it make sense to provide a macro that evaluates that condition? The code I saw earlier that I assume was checking this looked somewhat scary for after a clean-up.
I guess you mean e.g. in dbAccess.c:
+ if ((!pfl || (pfl->type=
+ paddr->
+ (prset = dbGetRset(paddr)) &&
+ prset->
+ status = prset->
+ } else {
offset = 0;
}
Yes, it probably isn't too obvious to the reader what goes on here. In a
later patch, I have used a local boolean variable a la
+ int pfl_has_copy = pfl && (pfl->type != dbfl_type_ref ||
pfl->u.r.dtor);
but I guess offering (and using) a macro here is the right thing. Will fix.
>> +<<<<<<< modules/
>
> I think the conflict markers shown here are really virtual, I believe they will disappear if/when we merge Dirk's branch that is marked as a prerequisite of this one.
The conflict is with a change you made on 7.0 since Dirk has created his
feature branch:
commit 25bb966cbc4be1f
Author: Andrew Johnson <email address hidden>
Date: Sat Mar 14 16:19:26 2020 -0500
Use the dbChannel*() accessor macros in the array filter code
instead of exposing the dbChannel innards unnecessarily.
I wasn't sure how to set up my branch so...
Ben Franksen (bfrk) wrote : Posted in a previous version of this proposal | # |
I removed the patch that changes the dbGetField API and added a new commit to address the point about "If the dtor is NULL, then this means the array data is still owned by a record".
The conflict is still there, pending a recommendation how to set up my branch properly.
Ben Franksen (bfrk) wrote : Posted in a previous version of this proposal | # |
Fixed a logical mistake in my last commit (rebased).
Ben Franksen (bfrk) wrote : Posted in a previous version of this proposal | # |
Since no-one came forward with a recommendation, I have now locally rebased all my changes such that they cleanly apply on branch 7.0. I did this by creating a new branch that forks off at the current HEAD of 7.0, then merging Dirk's branch dbChannelForDBLinks into that, and then replaying my changes, resolving the single conflict in the array filter implementation.
I have pushed this 'work' branch to my launchpad fork of epics base. All tests pass on all my intermediate versions.
Before I force-push the other branch refs (which are now on the work branch) to my fork (thereby updating all my merge requests), can somebody please tell me if this is what I should do?
Andrew Johnson (anj) wrote : Posted in a previous version of this proposal | # |
Core group 11/4: Will need some reworking after we have merged Dirk's other branch.
This will affect filters, breaking any external filter code (none known though).
The result looks cleaner so we like the idea in principle, but we can't merge it yet.
Ben Franksen (bfrk) wrote : Posted in a previous version of this proposal | # |
> The result looks cleaner so we like the idea in principle, but we can't merge
> it yet.
I can try to get this in shape (i.e. rebase it to the current HEAD). Are the prerequisites merged now? Should I move over to github (because of the automatic CI trigger there)?
Ben Franksen (bfrk) wrote : | # |
I have rebased an resubmitted. Unfortunately I am getting test failures now.
Ben Franksen (bfrk) wrote : | # |
This is the first failing test (dbCaLinkTest):
# fetch source.INP into source.BPTR
ok 21 - dbGetLink
ok 22 - nReq (1) == (long)num_min (1)
not ok 23 - array update
# 0 0 != 1
Not sure how to interpret this. My first blind guess is that the spec changed here, because
base now accepts empty (zero size) array requests?
Andrew Johnson (anj) wrote : Posted in a previous version of this proposal | # |
Hi Ben, yes I think everything else related has been merged now. Using GitHub instead of Launchpad does help with doing builds and running tests on our major architectures. Thanks!
Ben Franksen (bfrk) wrote : Posted in a previous version of this proposal | # |
Andrew Johnson (anj) wrote : | # |
That test passes on the current 7.0 branch, so something in your refactoring may have been developed based on an assumption that Dirk's array code invalidated:
# Link to a array numeric field
# TARGET=target CA,FTVL=
Warning: Registration already done.
#######
## EPICS R7.0.4.2-DEV
## Rev. R7.0.4.
#######
# fetch source.INP into source.BPTR
ok 21 - dbGetLink
ok 22 - nReq (1) == (long)num_min (1)
ok 23 - array update
Ben Franksen (bfrk) wrote : | # |
Am 12.01.21 um 19:04 schrieb Andrew Johnson:
> That test passes on the current 7.0 branch, so something in your
> refactoring may have been developed based on an assumption that
> Dirk's array code invalidated
Possibly. I vaguely remember that I had to jump through a few hoops to
get certain calls to fail on zero sized array requests, since the tests
specified it this way. I'll have to find those hoops and crawl back out...
mdavidsaver (mdavidsaver) wrote : | # |
FYI. in dbCaLinkTest, the tests in testArrayLink(
This is printed in earlier. eg. as Andrew shows:
> # Link to a array numeric field
> # TARGET=target CA,FTVL=
Andrew Johnson (anj) wrote : | # |
You can (almost always) tell from the TAP test numbers, I was quoting from the same result section as Ben and in this case the test numbers should always be the same. Some test programs can produce results in a different order when multiple threads can generate the test messages, but that's not too common.
Ben Franksen (bfrk) wrote : | # |
In dbCaLinkTest, the tests that fail are those where we check the resulting array data after a successful dbGetLink, except in one place in testArrayLink where we test after doing a putLink first. These are: the first two calls to checkArray in testArrayLink (but not the third) and the two calls to checkArrayDouble in testreTargetTyp
I spent yesterday staring at code differences (my changes and what changed in the 7.0 branch in the meantime). So far I couldn't find anything to explain why these tests succeed in my original version but fail now. This isn't easy to debug because of the CA layer in between with event queues and whatnot. So It is hard to guess the exact code path this takes (so I could insert printfs at these points).
Ben Franksen (bfrk) wrote : | # |
So far, looking at what changed between the current HEAD and the version I originally based my changes on, I have only considered changes under modules/database. I will now also consider the possibility that some changes outside of modules/database e.g. in modules/ca have invalidated my patch.
It is very unfortunate that some of the changes that happened in the meantime are bulk changes to things like copyright header comments because that means almost every file has been touched.
Andrew Johnson (anj) wrote : | # |
There were 2 commits from Dirk that affected modules/ca and might have had some effect:
8cc20393f1e34cf
a42197f0d670066
The first changed the dbr_size_n() macro to let it return a size of 0; the second makes it possible to put a zero-length array using the CA protocol.
I use Git's gitk GUI to examine and search the repo for these kinds of changes.
Have you worked out how the behavior of dbGetLink() and dbPutLink() have changed? Their functionality should be quite straight-forward.
Ben Franksen (bfrk) wrote : | # |
I gave up on comparing branches (i.e. finding out why it had worked half a year ago and fails now). Instead I concentrated on debugging the problem. I think I have found the correct fix and updated the github PR.
Ben Franksen (bfrk) wrote : | # |
The appveyor builds won't be finished until tomorrow but I don't believe there will be any problems. From my point of view this is now ready for merging.
Ben Franksen (bfrk) wrote : | # |
Andrew has expressed approval, at least regarding the general idea, and
asked me to bring it into shape for merging which I did. Is there
anything else I can or should do to make this receive a bit of attention?
Andrew Johnson (anj) wrote : | # |
That comment helped... I will see if we can review this in the Core Developers meeting tomorrow, thanks Ben.
Preview Diff
1 | diff --git a/modules/database/src/ioc/db/dbAccess.c b/modules/database/src/ioc/db/dbAccess.c |
2 | index d7e5d08..bac208f 100644 |
3 | --- a/modules/database/src/ioc/db/dbAccess.c |
4 | +++ b/modules/database/src/ioc/db/dbAccess.c |
5 | @@ -339,7 +339,7 @@ static void getOptions(DBADDR *paddr, char **poriginal, long *options, |
6 | dbCommon *pcommon; |
7 | char *pbuffer = *poriginal; |
8 | |
9 | - if (!pfl || pfl->type == dbfl_type_rec) |
10 | + if (!pfl) |
11 | field_type = paddr->field_type; |
12 | else |
13 | field_type = pfl->field_type; |
14 | @@ -349,7 +349,7 @@ static void getOptions(DBADDR *paddr, char **poriginal, long *options, |
15 | if( (*options) & DBR_STATUS ) { |
16 | unsigned short *pushort = (unsigned short *)pbuffer; |
17 | |
18 | - if (!pfl || pfl->type == dbfl_type_rec) { |
19 | + if (!pfl) { |
20 | *pushort++ = pcommon->stat; |
21 | *pushort++ = pcommon->sevr; |
22 | } else { |
23 | @@ -383,7 +383,7 @@ static void getOptions(DBADDR *paddr, char **poriginal, long *options, |
24 | if( (*options) & DBR_TIME ) { |
25 | epicsUInt32 *ptime = (epicsUInt32 *)pbuffer; |
26 | |
27 | - if (!pfl || pfl->type == dbfl_type_rec) { |
28 | + if (!pfl) { |
29 | *ptime++ = pcommon->time.secPastEpoch; |
30 | *ptime++ = pcommon->time.nsec; |
31 | } else { |
32 | @@ -904,22 +904,23 @@ long dbGet(DBADDR *paddr, short dbrType, |
33 | if (nRequest && *nRequest == 0) |
34 | return 0; |
35 | |
36 | - if (!pfl || pfl->type == dbfl_type_rec) { |
37 | + if (!pfl) { |
38 | field_type = paddr->field_type; |
39 | no_elements = capacity = paddr->no_elements; |
40 | - |
41 | - /* Update field info from record |
42 | - * may modify paddr->pfield |
43 | - */ |
44 | - if (paddr->pfldDes->special == SPC_DBADDR && |
45 | - (prset = dbGetRset(paddr)) && |
46 | - prset->get_array_info) { |
47 | - status = prset->get_array_info(paddr, &no_elements, &offset); |
48 | - } else |
49 | - offset = 0; |
50 | } else { |
51 | field_type = pfl->field_type; |
52 | no_elements = capacity = pfl->no_elements; |
53 | + } |
54 | + |
55 | + /* Update field info from record (if neccessary); |
56 | + * may modify paddr->pfield. |
57 | + */ |
58 | + if (!dbfl_has_copy(pfl) && |
59 | + paddr->pfldDes->special == SPC_DBADDR && |
60 | + (prset = dbGetRset(paddr)) && |
61 | + prset->get_array_info) { |
62 | + status = prset->get_array_info(paddr, &no_elements, &offset); |
63 | + } else { |
64 | offset = 0; |
65 | } |
66 | |
67 | @@ -951,7 +952,7 @@ long dbGet(DBADDR *paddr, short dbrType, |
68 | goto done; |
69 | } |
70 | |
71 | - if (!pfl || pfl->type == dbfl_type_rec) { |
72 | + if (!dbfl_has_copy(pfl)) { |
73 | status = dbFastGetConvertRoutine[field_type][dbrType] |
74 | (paddr->pfield, pbuf, paddr); |
75 | } else { |
76 | @@ -964,11 +965,9 @@ long dbGet(DBADDR *paddr, short dbrType, |
77 | |
78 | localAddr.field_type = pfl->field_type; |
79 | localAddr.field_size = pfl->field_size; |
80 | + /* not used by dbFastConvert: */ |
81 | localAddr.no_elements = pfl->no_elements; |
82 | - if (pfl->type == dbfl_type_val) |
83 | - localAddr.pfield = (char *) &pfl->u.v.field; |
84 | - else |
85 | - localAddr.pfield = (char *) pfl->u.r.field; |
86 | + localAddr.pfield = dbfl_pfield(pfl); |
87 | status = dbFastGetConvertRoutine[field_type][dbrType] |
88 | (localAddr.pfield, pbuf, &localAddr); |
89 | } |
90 | @@ -979,6 +978,8 @@ long dbGet(DBADDR *paddr, short dbrType, |
91 | if (nRequest) { |
92 | if (no_elements < *nRequest) |
93 | *nRequest = no_elements; |
94 | + if (capacity < *nRequest) |
95 | + *nRequest = capacity; |
96 | n = *nRequest; |
97 | } else { |
98 | n = 1; |
99 | @@ -995,8 +996,8 @@ long dbGet(DBADDR *paddr, short dbrType, |
100 | } |
101 | /* convert data into the caller's buffer */ |
102 | if (n <= 0) { |
103 | - ;/*do nothing*/ |
104 | - } else if (!pfl || pfl->type == dbfl_type_rec) { |
105 | + ; /*do nothing */ |
106 | + } else if (!dbfl_has_copy(pfl)) { |
107 | status = convert(paddr, pbuf, n, capacity, offset); |
108 | } else { |
109 | DBADDR localAddr = *paddr; /* Structure copy */ |
110 | @@ -1008,11 +1009,9 @@ long dbGet(DBADDR *paddr, short dbrType, |
111 | |
112 | localAddr.field_type = pfl->field_type; |
113 | localAddr.field_size = pfl->field_size; |
114 | + /* not used by dbConvert, it uses the passed capacity instead: */ |
115 | localAddr.no_elements = pfl->no_elements; |
116 | - if (pfl->type == dbfl_type_val) |
117 | - localAddr.pfield = (char *) &pfl->u.v.field; |
118 | - else |
119 | - localAddr.pfield = (char *) pfl->u.r.field; |
120 | + localAddr.pfield = dbfl_pfield(pfl); |
121 | status = convert(&localAddr, pbuf, n, capacity, offset); |
122 | } |
123 | |
124 | diff --git a/modules/database/src/ioc/db/dbChannel.c b/modules/database/src/ioc/db/dbChannel.c |
125 | index b6f53f7..c71d842 100644 |
126 | --- a/modules/database/src/ioc/db/dbChannel.c |
127 | +++ b/modules/database/src/ioc/db/dbChannel.c |
128 | @@ -52,14 +52,12 @@ typedef struct parseContext { |
129 | |
130 | static void *dbChannelFreeList; |
131 | static void *chFilterFreeList; |
132 | -static void *dbchStringFreeList; |
133 | |
134 | void dbChannelExit(void) |
135 | { |
136 | freeListCleanup(dbChannelFreeList); |
137 | freeListCleanup(chFilterFreeList); |
138 | - freeListCleanup(dbchStringFreeList); |
139 | - dbChannelFreeList = chFilterFreeList = dbchStringFreeList = NULL; |
140 | + dbChannelFreeList = chFilterFreeList = NULL; |
141 | } |
142 | |
143 | void dbChannelInit (void) |
144 | @@ -69,7 +67,6 @@ void dbChannelInit (void) |
145 | |
146 | freeListInitPvt(&dbChannelFreeList, sizeof(dbChannel), 128); |
147 | freeListInitPvt(&chFilterFreeList, sizeof(chFilter), 64); |
148 | - freeListInitPvt(&dbchStringFreeList, sizeof(epicsOldString), 128); |
149 | db_init_event_freelists(); |
150 | } |
151 | |
152 | @@ -449,28 +446,6 @@ static long parseArrayRange(dbChannel* chan, const char *pname, const char **ppn |
153 | return status; |
154 | } |
155 | |
156 | -/* Stolen from dbAccess.c: */ |
157 | -static short mapDBFToDBR[DBF_NTYPES] = |
158 | - { |
159 | - /* DBF_STRING => */DBR_STRING, |
160 | - /* DBF_CHAR => */DBR_CHAR, |
161 | - /* DBF_UCHAR => */DBR_UCHAR, |
162 | - /* DBF_SHORT => */DBR_SHORT, |
163 | - /* DBF_USHORT => */DBR_USHORT, |
164 | - /* DBF_LONG => */DBR_LONG, |
165 | - /* DBF_ULONG => */DBR_ULONG, |
166 | - /* DBF_INT64 => */DBR_INT64, |
167 | - /* DBF_UINT64 => */DBR_UINT64, |
168 | - /* DBF_FLOAT => */DBR_FLOAT, |
169 | - /* DBF_DOUBLE => */DBR_DOUBLE, |
170 | - /* DBF_ENUM, => */DBR_ENUM, |
171 | - /* DBF_MENU, => */DBR_ENUM, |
172 | - /* DBF_DEVICE => */DBR_ENUM, |
173 | - /* DBF_INLINK => */DBR_STRING, |
174 | - /* DBF_OUTLINK => */DBR_STRING, |
175 | - /* DBF_FWDLINK => */DBR_STRING, |
176 | - /* DBF_NOACCESS => */DBR_NOACCESS }; |
177 | - |
178 | dbChannel * dbChannelCreate(const char *name) |
179 | { |
180 | const char *pname = name; |
181 | @@ -743,37 +718,24 @@ void dbChannelDelete(dbChannel *chan) |
182 | freeListFree(dbChannelFreeList, chan); |
183 | } |
184 | |
185 | -static void freeArray(db_field_log *pfl) { |
186 | - if (pfl->field_type == DBF_STRING && pfl->no_elements == 1) { |
187 | - freeListFree(dbchStringFreeList, pfl->u.r.field); |
188 | - } else { |
189 | - free(pfl->u.r.field); |
190 | - } |
191 | -} |
192 | - |
193 | -void dbChannelMakeArrayCopy(void *pvt, db_field_log *pfl, dbChannel *chan) |
194 | +/* |
195 | + * Helper function to adjust no_elements, offset, and pfield |
196 | + * when copying an array from a record. |
197 | + */ |
198 | +void dbChannelGetArrayInfo(dbChannel *chan, |
199 | + void **pfield, long *no_elements, long *offset) |
200 | { |
201 | - void *p; |
202 | - struct dbCommon *prec = dbChannelRecord(chan); |
203 | - |
204 | - if (pfl->type != dbfl_type_rec) return; |
205 | - |
206 | - pfl->type = dbfl_type_ref; |
207 | - pfl->stat = prec->stat; |
208 | - pfl->sevr = prec->sevr; |
209 | - pfl->time = prec->time; |
210 | - pfl->field_type = chan->addr.field_type; |
211 | - pfl->no_elements = chan->addr.no_elements; |
212 | - pfl->field_size = chan->addr.field_size; |
213 | - pfl->u.r.dtor = freeArray; |
214 | - pfl->u.r.pvt = pvt; |
215 | - if (pfl->field_type == DBF_STRING && pfl->no_elements == 1) { |
216 | - p = freeListCalloc(dbchStringFreeList); |
217 | - } else { |
218 | - p = calloc(pfl->no_elements, pfl->field_size); |
219 | + rset *prset; |
220 | + if (dbChannelSpecial(chan) == SPC_DBADDR && |
221 | + (prset = dbGetRset(&chan->addr)) && |
222 | + prset->get_array_info) |
223 | + { |
224 | + void *pfieldsave = dbChannelField(chan); |
225 | + /* it is expected that this call always succeeds */ |
226 | + prset->get_array_info(&chan->addr, no_elements, offset); |
227 | + *pfield = dbChannelField(chan); |
228 | + dbChannelField(chan) = pfieldsave; |
229 | } |
230 | - if (p) dbGet(&chan->addr, mapDBFToDBR[pfl->field_type], p, NULL, &pfl->no_elements, NULL); |
231 | - pfl->u.r.field = p; |
232 | } |
233 | |
234 | /* FIXME: Do these belong in a different file? */ |
235 | diff --git a/modules/database/src/ioc/db/dbChannel.h b/modules/database/src/ioc/db/dbChannel.h |
236 | index 1ca02bb..ec86e9e 100644 |
237 | --- a/modules/database/src/ioc/db/dbChannel.h |
238 | +++ b/modules/database/src/ioc/db/dbChannel.h |
239 | @@ -65,8 +65,8 @@ typedef struct dbChannel { |
240 | /* Prototype for the channel event function that is called in filter stacks |
241 | * |
242 | * When invoked the scan lock for the record associated with 'chan' _may_ be locked. |
243 | - * If pLog->type==dbfl_type_rec then dbScanLock() must be called before copying |
244 | - * data out of the associated record. |
245 | + * Unless dbfl_has_copy(pLog), it must call dbScanLock before accessing the data, |
246 | + * as this indicates the data is still owned by the record. |
247 | * |
248 | * This function has ownership of the field log pLog, if it wishes to discard |
249 | * this update it should free the field log with db_delete_field_log() and |
250 | @@ -225,7 +225,8 @@ DBCORE_API void dbRegisterFilter(const char *key, const chFilterIf *fif, void *p |
251 | DBCORE_API db_field_log* dbChannelRunPreChain(dbChannel *chan, db_field_log *pLogIn); |
252 | DBCORE_API db_field_log* dbChannelRunPostChain(dbChannel *chan, db_field_log *pLogIn); |
253 | DBCORE_API const chFilterPlugin * dbFindFilter(const char *key, size_t len); |
254 | -DBCORE_API void dbChannelMakeArrayCopy(void *pvt, db_field_log *pfl, dbChannel *chan); |
255 | +DBCORE_API void dbChannelGetArrayInfo(dbChannel *chan, |
256 | + void **pfield, long *no_elements, long *offset); |
257 | |
258 | #ifdef __cplusplus |
259 | } |
260 | diff --git a/modules/database/src/ioc/db/dbEvent.c b/modules/database/src/ioc/db/dbEvent.c |
261 | index 437cb6d..1db01f2 100644 |
262 | --- a/modules/database/src/ioc/db/dbEvent.c |
263 | +++ b/modules/database/src/ioc/db/dbEvent.c |
264 | @@ -668,27 +668,21 @@ int db_post_extra_labor (dbEventCtx ctx) |
265 | return DB_EVENT_OK; |
266 | } |
267 | |
268 | -/* |
269 | - * DB_CREATE_EVENT_LOG() |
270 | - * |
271 | - * NOTE: This assumes that the db scan lock is already applied |
272 | - * (as it copies data from the record) |
273 | - */ |
274 | -db_field_log* db_create_event_log (struct evSubscrip *pevent) |
275 | +static db_field_log* db_create_field_log (struct dbChannel *chan, int use_val) |
276 | { |
277 | db_field_log *pLog = (db_field_log *) freeListCalloc(dbevFieldLogFreeList); |
278 | |
279 | if (pLog) { |
280 | - struct dbChannel *chan = pevent->chan; |
281 | struct dbCommon *prec = dbChannelRecord(chan); |
282 | - pLog->ctx = dbfl_context_event; |
283 | - if (pevent->useValque) { |
284 | + pLog->stat = prec->stat; |
285 | + pLog->sevr = prec->sevr; |
286 | + pLog->time = prec->time; |
287 | + pLog->field_type = dbChannelFieldType(chan); |
288 | + pLog->field_size = dbChannelFieldSize(chan); |
289 | + pLog->no_elements = dbChannelElements(chan); |
290 | + |
291 | + if (use_val) { |
292 | pLog->type = dbfl_type_val; |
293 | - pLog->stat = prec->stat; |
294 | - pLog->sevr = prec->sevr; |
295 | - pLog->time = prec->time; |
296 | - pLog->field_type = dbChannelFieldType(chan); |
297 | - pLog->no_elements = dbChannelElements(chan); |
298 | /* |
299 | * use memcpy to avoid a bus error on |
300 | * union copy of char in the db at an odd |
301 | @@ -698,23 +692,46 @@ db_field_log* db_create_event_log (struct evSubscrip *pevent) |
302 | dbChannelField(chan), |
303 | dbChannelFieldSize(chan)); |
304 | } else { |
305 | - pLog->type = dbfl_type_rec; |
306 | + pLog->type = dbfl_type_ref; |
307 | + |
308 | + /* don't make a copy yet, just reference the field value */ |
309 | + pLog->u.r.field = dbChannelField(chan); |
310 | + /* indicate field value still owned by record */ |
311 | + pLog->u.r.dtor = NULL; |
312 | + /* no private data yet, may be set by a filter */ |
313 | + pLog->u.r.pvt = NULL; |
314 | } |
315 | } |
316 | return pLog; |
317 | } |
318 | |
319 | /* |
320 | + * DB_CREATE_EVENT_LOG() |
321 | + * |
322 | + * NOTE: This assumes that the db scan lock is already applied |
323 | + * (as it calls rset->get_array_info) |
324 | + */ |
325 | +db_field_log* db_create_event_log (struct evSubscrip *pevent) |
326 | +{ |
327 | + db_field_log *pLog = db_create_field_log(pevent->chan, pevent->useValque); |
328 | + if (pLog) { |
329 | + pLog->ctx = dbfl_context_event; |
330 | + } |
331 | + return pLog; |
332 | +} |
333 | + |
334 | +/* |
335 | * DB_CREATE_READ_LOG() |
336 | * |
337 | */ |
338 | db_field_log* db_create_read_log (struct dbChannel *chan) |
339 | { |
340 | - db_field_log *pLog = (db_field_log *) freeListCalloc(dbevFieldLogFreeList); |
341 | - |
342 | + db_field_log *pLog = db_create_field_log(chan, |
343 | + dbChannelElements(chan) == 1 && |
344 | + dbChannelSpecial(chan) != SPC_DBADDR && |
345 | + dbChannelFieldSize(chan) <= sizeof(union native_value)); |
346 | if (pLog) { |
347 | pLog->ctx = dbfl_context_read; |
348 | - pLog->type = dbfl_type_rec; |
349 | } |
350 | return pLog; |
351 | } |
352 | @@ -738,20 +755,6 @@ static void db_queue_event_log (evSubscrip *pevent, db_field_log *pLog) |
353 | LOCKEVQUE (ev_que); |
354 | |
355 | /* |
356 | - * if we have an event on the queue and both the last |
357 | - * event on the queue and the current event are emtpy |
358 | - * (i.e. of type dbfl_type_rec), simply ignore duplicate |
359 | - * events (saving empty events serves no purpose) |
360 | - */ |
361 | - if (pevent->npend > 0u && |
362 | - (*pevent->pLastLog)->type == dbfl_type_rec && |
363 | - pLog->type == dbfl_type_rec) { |
364 | - db_delete_field_log(pLog); |
365 | - UNLOCKEVQUE (ev_que); |
366 | - return; |
367 | - } |
368 | - |
369 | - /* |
370 | * add to task local event que |
371 | */ |
372 | |
373 | diff --git a/modules/database/src/ioc/db/dbExtractArray.c b/modules/database/src/ioc/db/dbExtractArray.c |
374 | index a7dcf4d..197e66c 100644 |
375 | --- a/modules/database/src/ioc/db/dbExtractArray.c |
376 | +++ b/modules/database/src/ioc/db/dbExtractArray.c |
377 | @@ -14,11 +14,12 @@ |
378 | /* |
379 | * Author: Ralph Lange <Ralph.Lange@bessy.de> |
380 | * |
381 | - * based on dbConvert.c |
382 | + * based on dbConvert.c, see copyNoConvert |
383 | * written by: Bob Dalesio, Marty Kraimer |
384 | */ |
385 | |
386 | #include <string.h> |
387 | +#include <assert.h> |
388 | |
389 | #include "epicsTypes.h" |
390 | |
391 | @@ -26,61 +27,30 @@ |
392 | #include "dbAddr.h" |
393 | #include "dbExtractArray.h" |
394 | |
395 | -void dbExtractArrayFromRec(const dbAddr *paddr, void *pto, |
396 | - long nRequest, long no_elements, long offset, long increment) |
397 | +void dbExtractArray(const void *pfrom, void *pto, short field_size, |
398 | + long nRequest, long no_elements, long offset, long increment) |
399 | { |
400 | char *pdst = (char *) pto; |
401 | - char *psrc = (char *) paddr->pfield; |
402 | - long nUpperPart; |
403 | - int i; |
404 | - short srcSize = paddr->field_size; |
405 | - short dstSize = srcSize; |
406 | - char isString = (paddr->field_type == DBF_STRING); |
407 | + const char *psrc = (char *) pfrom; |
408 | |
409 | - if (nRequest > no_elements) nRequest = no_elements; |
410 | - if (isString && srcSize > MAX_STRING_SIZE) dstSize = MAX_STRING_SIZE; |
411 | + /* assert preconditions */ |
412 | + assert(nRequest >= 0); |
413 | + assert(no_elements >= 0); |
414 | + assert(increment > 0); |
415 | + assert(0 <= offset); |
416 | + assert(offset < no_elements); |
417 | |
418 | - if (increment == 1 && dstSize == srcSize) { |
419 | - nUpperPart = nRequest < no_elements - offset ? nRequest : no_elements - offset; |
420 | - memcpy(pdst, &psrc[offset * srcSize], dstSize * nUpperPart); |
421 | + if (increment == 1) { |
422 | + long nUpperPart = |
423 | + nRequest < no_elements - offset ? nRequest : no_elements - offset; |
424 | + memcpy(pdst, psrc + (offset * field_size), field_size * nUpperPart); |
425 | if (nRequest > nUpperPart) |
426 | - memcpy(&pdst[dstSize * nUpperPart], psrc, dstSize * (nRequest - nUpperPart)); |
427 | - if (isString) |
428 | - for (i = 1; i <= nRequest; i++) |
429 | - pdst[dstSize*i-1] = '\0'; |
430 | + memcpy(pdst + (field_size * nUpperPart), psrc, |
431 | + field_size * (nRequest - nUpperPart)); |
432 | } else { |
433 | - for (; nRequest > 0; nRequest--, pdst += dstSize, offset += increment) { |
434 | + for (; nRequest > 0; nRequest--, pdst += field_size, offset += increment) { |
435 | offset %= no_elements; |
436 | - memcpy(pdst, &psrc[offset*srcSize], dstSize); |
437 | - if (isString) pdst[dstSize-1] = '\0'; |
438 | - } |
439 | - } |
440 | -} |
441 | - |
442 | -void dbExtractArrayFromBuf(const void *pfrom, void *pto, |
443 | - short field_size, short field_type, |
444 | - long nRequest, long no_elements, long offset, long increment) |
445 | -{ |
446 | - char *pdst = (char *) pto; |
447 | - char *psrc = (char *) pfrom; |
448 | - int i; |
449 | - short srcSize = field_size; |
450 | - short dstSize = srcSize; |
451 | - char isString = (field_type == DBF_STRING); |
452 | - |
453 | - if (nRequest > no_elements) nRequest = no_elements; |
454 | - if (offset > no_elements - 1) offset = no_elements - 1; |
455 | - if (isString && dstSize >= MAX_STRING_SIZE) dstSize = MAX_STRING_SIZE - 1; |
456 | - |
457 | - if (increment == 1) { |
458 | - memcpy(pdst, &psrc[offset * srcSize], dstSize * nRequest); |
459 | - if (isString) |
460 | - for (i = 1; i <= nRequest; i++) |
461 | - pdst[dstSize*i] = '\0'; |
462 | - } else { |
463 | - for (; nRequest > 0; nRequest--, pdst += srcSize, offset += increment) { |
464 | - memcpy(pdst, &psrc[offset*srcSize], dstSize); |
465 | - if (isString) pdst[dstSize] = '\0'; |
466 | + memcpy(pdst, psrc + (offset * field_size), field_size); |
467 | } |
468 | } |
469 | } |
470 | diff --git a/modules/database/src/ioc/db/dbExtractArray.h b/modules/database/src/ioc/db/dbExtractArray.h |
471 | index b44bd15..9755ac5 100644 |
472 | --- a/modules/database/src/ioc/db/dbExtractArray.h |
473 | +++ b/modules/database/src/ioc/db/dbExtractArray.h |
474 | @@ -22,11 +22,36 @@ |
475 | extern "C" { |
476 | #endif |
477 | |
478 | -epicsShareFunc void dbExtractArrayFromRec(const DBADDR *paddr, void *pto, |
479 | - long nRequest, long no_elements, long offset, long increment); |
480 | -epicsShareFunc void dbExtractArrayFromBuf(const void *pfrom, void *pto, |
481 | - short field_size, short field_type, |
482 | - long nRequest, long no_elements, long offset, long increment); |
483 | +/** @brief Make a copy of parts of an array. |
484 | + * |
485 | + * The source array may or may not be a record field. |
486 | + * |
487 | + * The increment parameter is used to support array filters; it |
488 | + * means: copy only every increment'th element, starting at offset. |
489 | + * |
490 | + * The offset and no_elements parameters are used to support the |
491 | + * circular buffer feature of record fields: elements before offset |
492 | + * are treated as if they came right after no_elements. |
493 | + * |
494 | + * This function does not do any conversion on the array elements. |
495 | + * |
496 | + * Preconditions: |
497 | + * nRequest >= 0, no_elements >= 0, increment > 0 |
498 | + * 0 <= offset < no_elements |
499 | + * pto points to a buffer with at least field_size*nRequest bytes |
500 | + * pfrom points to a buffer with exactly field_size*no_elements bytes |
501 | + * |
502 | + * @param pfrom Pointer to source array. |
503 | + * @param pto Pointer to target array. |
504 | + * @param field_size Size of an array element. |
505 | + * @param nRequest Number of elements to copy. |
506 | + * @param no_elements Number of elements in source array. |
507 | + * @param offset Wrap-around point in source array. |
508 | + * @param increment Copy only every increment'th element. |
509 | + */ |
510 | +epicsShareFunc void dbExtractArray(const void *pfrom, void *pto, |
511 | + short field_size, long nRequest, long no_elements, long offset, |
512 | + long increment); |
513 | |
514 | #ifdef __cplusplus |
515 | } |
516 | diff --git a/modules/database/src/ioc/db/db_field_log.h b/modules/database/src/ioc/db/db_field_log.h |
517 | index 5a40f5f..e517d52 100644 |
518 | --- a/modules/database/src/ioc/db/db_field_log.h |
519 | +++ b/modules/database/src/ioc/db/db_field_log.h |
520 | @@ -57,20 +57,31 @@ union native_value { |
521 | struct db_field_log; |
522 | typedef void (dbfl_freeFunc)(struct db_field_log *pfl); |
523 | |
524 | -/* Types of db_field_log: rec = use record, val = val inside, ref = reference inside */ |
525 | +/* |
526 | + * A db_field_log has one of two types: |
527 | + * |
528 | + * dbfl_type_ref - Reference to value |
529 | + * Used for variable size (array) data types. Meta-data |
530 | + * is stored in the field log, but value data is stored externally. |
531 | + * Only the dbfl_ref side of the data union is valid. |
532 | + * |
533 | + * dbfl_type_val - Internal value |
534 | + * Used to store small scalar data. Meta-data and value are |
535 | + * present in this structure and no external references are used. |
536 | + * Only the dbfl_val side of the data union is valid. |
537 | + */ |
538 | typedef enum dbfl_type { |
539 | - dbfl_type_rec = 0, |
540 | dbfl_type_val, |
541 | dbfl_type_ref |
542 | } dbfl_type; |
543 | |
544 | /* Context of db_field_log: event = subscription update, read = read reply */ |
545 | typedef enum dbfl_context { |
546 | - dbfl_context_read = 0, |
547 | + dbfl_context_read, |
548 | dbfl_context_event |
549 | } dbfl_context; |
550 | |
551 | -#define dbflTypeStr(t) (t==dbfl_type_val?"val":t==dbfl_type_rec?"rec":"ref") |
552 | +#define dbflTypeStr(t) (t==dbfl_type_val?"val":"ref") |
553 | |
554 | struct dbfl_val { |
555 | union native_value field; /* Field value */ |
556 | @@ -82,6 +93,8 @@ struct dbfl_val { |
557 | * db_delete_field_log(). Any code which changes a dbfl_type_ref |
558 | * field log to another type, or to reference different data, |
559 | * must explicitly call the dtor function. |
560 | + * If the dtor is NULL and no_elements > 0, then this means the array |
561 | + * data is still owned by a record. See the macro dbfl_has_copy below. |
562 | */ |
563 | struct dbfl_ref { |
564 | dbfl_freeFunc *dtor; /* Callback to free filter-allocated resources */ |
565 | @@ -89,8 +102,11 @@ struct dbfl_ref { |
566 | void *field; /* Field value */ |
567 | }; |
568 | |
569 | +/* |
570 | + * Note that field_size may be larger than MAX_STRING_SIZE. |
571 | + */ |
572 | typedef struct db_field_log { |
573 | - unsigned int type:2; /* type (union) selector */ |
574 | + unsigned int type:1; /* type (union) selector */ |
575 | /* ctx is used for all types */ |
576 | unsigned int ctx:1; /* context (operation type) */ |
577 | /* the following are used for value and reference types */ |
578 | @@ -98,8 +114,8 @@ typedef struct db_field_log { |
579 | unsigned short stat; /* Alarm Status */ |
580 | unsigned short sevr; /* Alarm Severity */ |
581 | short field_type; /* DBF type of data */ |
582 | - short field_size; /* Data size */ |
583 | - long no_elements; /* No of array elements */ |
584 | + short field_size; /* Size of a single element */ |
585 | + long no_elements; /* No of valid array elements */ |
586 | union { |
587 | struct dbfl_val v; |
588 | struct dbfl_ref r; |
589 | @@ -107,27 +123,19 @@ typedef struct db_field_log { |
590 | } db_field_log; |
591 | |
592 | /* |
593 | - * A db_field_log will in one of three types: |
594 | - * |
595 | - * dbfl_type_rec - Reference to record |
596 | - * The field log stores no data itself. Data must instead be taken |
597 | - * via the dbChannel* which must always be provided when along |
598 | - * with the field log. |
599 | - * For this type only the 'type' and 'ctx' members are used. |
600 | - * |
601 | - * dbfl_type_ref - Reference to outside value |
602 | - * Used for variable size (array) data types. Meta-data |
603 | - * is stored in the field log, but value data is stored externally |
604 | - * (see struct dbfl_ref). |
605 | - * For this type all meta-data members are used. The dbfl_ref side of the |
606 | - * data union is used. |
607 | - * |
608 | - * dbfl_type_val - Internal value |
609 | - * Used to store small scalar data. Meta-data and value are |
610 | - * present in this structure and no external references are used. |
611 | - * For this type all meta-data members are used. The dbfl_val side of the |
612 | - * data union is used. |
613 | + * Whether a db_field_log* owns the field data. If this is the case, then the |
614 | + * db_field_log is fully decoupled from the record: there is no need to lock |
615 | + * the record when accessing the data, nor to call any rset methods (like |
616 | + * get_array_info) because this has already been done when we made the copy. A |
617 | + * special case here is that of no (remaining) data (i.e. no_elements==0). In |
618 | + * this case, making a copy is redundant, so we have no dtor. But conceptually |
619 | + * the db_field_log still owns the (empty) data. |
620 | */ |
621 | +#define dbfl_has_copy(p)\ |
622 | + ((p) && ((p)->type==dbfl_type_val || (p)->u.r.dtor || (p)->no_elements==0)) |
623 | + |
624 | +#define dbfl_pfield(p)\ |
625 | + ((p)->type==dbfl_type_val ? &p->u.v.field : p->u.r.field) |
626 | |
627 | #ifdef __cplusplus |
628 | } |
629 | diff --git a/modules/database/src/std/filters/arr.c b/modules/database/src/std/filters/arr.c |
630 | index 8138480..ffe3fce 100644 |
631 | --- a/modules/database/src/std/filters/arr.c |
632 | +++ b/modules/database/src/std/filters/arr.c |
633 | @@ -13,16 +13,14 @@ |
634 | |
635 | #include <stdio.h> |
636 | |
637 | -#include <freeList.h> |
638 | -#include <dbAccess.h> |
639 | -#include <dbExtractArray.h> |
640 | -#include <db_field_log.h> |
641 | -#include <dbLock.h> |
642 | -#include <recSup.h> |
643 | -#include <epicsExit.h> |
644 | -#include <special.h> |
645 | -#include <chfPlugin.h> |
646 | -#include <epicsExport.h> |
647 | +#include "chfPlugin.h" |
648 | +#include "dbAccessDefs.h" |
649 | +#include "dbExtractArray.h" |
650 | +#include "db_field_log.h" |
651 | +#include "dbLock.h" |
652 | +#include "epicsExit.h" |
653 | +#include "freeList.h" |
654 | +#include "epicsExport.h" |
655 | |
656 | typedef struct myStruct { |
657 | epicsInt32 start; |
658 | @@ -46,6 +44,8 @@ static void * allocPvt(void) |
659 | myStruct *my = (myStruct*) freeListCalloc(myStructFreeList); |
660 | if (!my) return NULL; |
661 | |
662 | + /* defaults */ |
663 | + my->start = 0; |
664 | my->incr = 1; |
665 | my->end = -1; |
666 | return (void *) my; |
667 | @@ -77,8 +77,6 @@ static void freeArray(db_field_log *pfl) |
668 | static long wrapArrayIndices(long *start, const long increment, long *end, |
669 | const long no_elements) |
670 | { |
671 | - long len = 0; |
672 | - |
673 | if (*start < 0) *start = no_elements + *start; |
674 | if (*start < 0) *start = 0; |
675 | if (*start > no_elements) *start = no_elements; |
676 | @@ -87,85 +85,53 @@ static long wrapArrayIndices(long *start, const long increment, long *end, |
677 | if (*end < 0) *end = 0; |
678 | if (*end >= no_elements) *end = no_elements - 1; |
679 | |
680 | - if (*end - *start >= 0) len = 1 + (*end - *start) / increment; |
681 | - return len; |
682 | + if (*end - *start >= 0) |
683 | + return 1 + (*end - *start) / increment; |
684 | + else |
685 | + return 0; |
686 | } |
687 | |
688 | static db_field_log* filter(void* pvt, dbChannel *chan, db_field_log *pfl) |
689 | { |
690 | myStruct *my = (myStruct*) pvt; |
691 | - struct dbCommon *prec; |
692 | - rset *prset; |
693 | + int must_lock; |
694 | long start = my->start; |
695 | long end = my->end; |
696 | - long nTarget = 0; |
697 | + long nTarget; |
698 | + void *pTarget; |
699 | long offset = 0; |
700 | - long nSource = dbChannelElements(chan); |
701 | - long capacity = nSource; |
702 | - void *pdst; |
703 | + long nSource = pfl->no_elements; |
704 | + void *pSource = pfl->u.r.field; |
705 | |
706 | switch (pfl->type) { |
707 | case dbfl_type_val: |
708 | - /* Only filter arrays */ |
709 | - break; |
710 | - |
711 | - case dbfl_type_rec: |
712 | - /* Extract from record */ |
713 | - if (dbChannelSpecial(chan) == SPC_DBADDR && |
714 | - nSource > 1 && |
715 | - (prset = dbGetRset(&chan->addr)) && |
716 | - prset->get_array_info) |
717 | - { |
718 | - void *pfieldsave = dbChannelField(chan); |
719 | - prec = dbChannelRecord(chan); |
720 | - dbScanLock(prec); |
721 | - prset->get_array_info(&chan->addr, &nSource, &offset); |
722 | - nTarget = wrapArrayIndices(&start, my->incr, &end, nSource); |
723 | - pfl->type = dbfl_type_ref; |
724 | - pfl->stat = prec->stat; |
725 | - pfl->sevr = prec->sevr; |
726 | - pfl->time = prec->time; |
727 | - pfl->field_type = dbChannelFieldType(chan); |
728 | - pfl->field_size = dbChannelFieldSize(chan); |
729 | - pfl->no_elements = nTarget; |
730 | - if (nTarget) { |
731 | - pdst = freeListCalloc(my->arrayFreeList); |
732 | - if (pdst) { |
733 | - pfl->u.r.dtor = freeArray; |
734 | - pfl->u.r.pvt = my->arrayFreeList; |
735 | - offset = (offset + start) % dbChannelElements(chan); |
736 | - dbExtractArrayFromRec(&chan->addr, pdst, nTarget, capacity, |
737 | - offset, my->incr); |
738 | - pfl->u.r.field = pdst; |
739 | - } |
740 | - } |
741 | - dbScanUnlock(prec); |
742 | - dbChannelField(chan) = pfieldsave; |
743 | - } |
744 | + /* TODO Treat scalars as arrays with 1 element */ |
745 | break; |
746 | |
747 | - /* Extract from buffer */ |
748 | case dbfl_type_ref: |
749 | - pdst = NULL; |
750 | - nSource = pfl->no_elements; |
751 | - nTarget = wrapArrayIndices(&start, my->incr, &end, nSource); |
752 | - pfl->no_elements = nTarget; |
753 | - if (nTarget) { |
754 | - /* Copy the data out */ |
755 | - void *psrc = pfl->u.r.field; |
756 | - |
757 | - pdst = freeListCalloc(my->arrayFreeList); |
758 | - if (!pdst) break; |
759 | - offset = start; |
760 | - dbExtractArrayFromBuf(psrc, pdst, pfl->field_size, pfl->field_type, |
761 | - nTarget, nSource, offset, my->incr); |
762 | + must_lock = !pfl->u.r.dtor; |
763 | + if (must_lock) { |
764 | + dbScanLock(dbChannelRecord(chan)); |
765 | + dbChannelGetArrayInfo(chan, &pSource, &nSource, &offset); |
766 | } |
767 | - if (pfl->u.r.dtor) pfl->u.r.dtor(pfl); |
768 | - if (nTarget) { |
769 | + nTarget = wrapArrayIndices(&start, my->incr, &end, nSource); |
770 | + if (nTarget > 0) { |
771 | + /* copy the data */ |
772 | + pTarget = freeListCalloc(my->arrayFreeList); |
773 | + if (!pTarget) break; |
774 | + /* must do the wrap-around with the original no_elements */ |
775 | + offset = (offset + start) % pfl->no_elements; |
776 | + dbExtractArray(pSource, pTarget, pfl->field_size, |
777 | + nTarget, pfl->no_elements, offset, my->incr); |
778 | + if (pfl->u.r.dtor) pfl->u.r.dtor(pfl); |
779 | + pfl->u.r.field = pTarget; |
780 | pfl->u.r.dtor = freeArray; |
781 | pfl->u.r.pvt = my->arrayFreeList; |
782 | - pfl->u.r.field = pdst; |
783 | } |
784 | + /* adjust no_elements (even if zero elements remain) */ |
785 | + pfl->no_elements = nTarget; |
786 | + if (must_lock) |
787 | + dbScanUnlock(dbChannelRecord(chan)); |
788 | break; |
789 | } |
790 | return pfl; |
791 | diff --git a/modules/database/src/std/filters/ts.c b/modules/database/src/std/filters/ts.c |
792 | index 8a07c3f..0feadb0 100644 |
793 | --- a/modules/database/src/std/filters/ts.c |
794 | +++ b/modules/database/src/std/filters/ts.c |
795 | @@ -12,21 +12,44 @@ |
796 | */ |
797 | |
798 | #include <stdio.h> |
799 | +#include <stdlib.h> |
800 | +#include <string.h> |
801 | |
802 | -#include <chfPlugin.h> |
803 | -#include <dbLock.h> |
804 | -#include <db_field_log.h> |
805 | -#include <epicsExport.h> |
806 | +#include "chfPlugin.h" |
807 | +#include "db_field_log.h" |
808 | +#include "dbExtractArray.h" |
809 | +#include "dbLock.h" |
810 | +#include "epicsExport.h" |
811 | + |
812 | +/* |
813 | + * The size of the data is different for each channel, and can even |
814 | + * change at runtime, so a freeList doesn't make much sense here. |
815 | + */ |
816 | +static void freeArray(db_field_log *pfl) { |
817 | + free(pfl->u.r.field); |
818 | +} |
819 | |
820 | static db_field_log* filter(void* pvt, dbChannel *chan, db_field_log *pfl) { |
821 | epicsTimeStamp now; |
822 | epicsTimeGetCurrent(&now); |
823 | |
824 | - /* If string or array, must make a copy (to ensure coherence between time and data) */ |
825 | - if (pfl->type == dbfl_type_rec) { |
826 | - dbScanLock(dbChannelRecord(chan)); |
827 | - dbChannelMakeArrayCopy(pvt, pfl, chan); |
828 | - dbScanUnlock(dbChannelRecord(chan)); |
829 | + /* If reference and not already copied, |
830 | + must make a copy (to ensure coherence between time and data) */ |
831 | + if (pfl->type == dbfl_type_ref && !pfl->u.r.dtor) { |
832 | + void *pTarget = calloc(pfl->no_elements, pfl->field_size); |
833 | + void *pSource = pfl->u.r.field; |
834 | + if (pTarget) { |
835 | + long offset = 0; |
836 | + long nSource = pfl->no_elements; |
837 | + dbScanLock(dbChannelRecord(chan)); |
838 | + dbChannelGetArrayInfo(chan, &pSource, &nSource, &offset); |
839 | + dbExtractArray(pSource, pTarget, pfl->field_size, |
840 | + nSource, pfl->no_elements, offset, 1); |
841 | + pfl->u.r.field = pTarget; |
842 | + pfl->u.r.dtor = freeArray; |
843 | + pfl->u.r.pvt = pvt; |
844 | + dbScanUnlock(dbChannelRecord(chan)); |
845 | + } |
846 | } |
847 | |
848 | pfl->time = now; |
849 | diff --git a/modules/database/test/ioc/db/dbChArrTest.cpp b/modules/database/test/ioc/db/dbChArrTest.cpp |
850 | index 90702b5..9965852 100644 |
851 | --- a/modules/database/test/ioc/db/dbChArrTest.cpp |
852 | +++ b/modules/database/test/ioc/db/dbChArrTest.cpp |
853 | @@ -131,7 +131,7 @@ static void check(short dbr_type) { |
854 | memset(buf, 0, sizeof(buf)); \ |
855 | (void) dbPutField(&offaddr, DBR_LONG, &off, 1); \ |
856 | pfl = db_create_read_log(pch); \ |
857 | - testOk(pfl && pfl->type == dbfl_type_rec, "Valid pfl, type = rec"); \ |
858 | + testOk(pfl && pfl->type == dbfl_type_ref, "Valid pfl, type = ref"); \ |
859 | testOk(!dbChannelGetField(pch, DBR_LONG, buf, NULL, &req, pfl), "Got Field value"); \ |
860 | testOk(req == Size, "Got %ld elements (expected %d)", req, Size); \ |
861 | if (!testOk(!memcmp(buf, Expected, sizeof(Expected)), "Data correct")) \ |
862 | diff --git a/modules/database/test/std/filters/arrTest.cpp b/modules/database/test/std/filters/arrTest.cpp |
863 | index bd83bd8..dfbbf46 100644 |
864 | --- a/modules/database/test/std/filters/arrTest.cpp |
865 | +++ b/modules/database/test/std/filters/arrTest.cpp |
866 | @@ -73,9 +73,9 @@ static int fl_equals_array(short type, const db_field_log *pfl1, void *p2) { |
867 | } |
868 | break; |
869 | case DBR_STRING: |
870 | - if (strtol(&((const char*)pfl1->u.r.field)[i*MAX_STRING_SIZE], NULL, 0) != ((epicsInt32*)p2)[i]) { |
871 | + if (strtol(&((const char*)pfl1->u.r.field)[i*pfl1->field_size], NULL, 0) != ((epicsInt32*)p2)[i]) { |
872 | testDiag("at index=%d: field log has '%s', should be '%d'", |
873 | - i, &((const char*)pfl1->u.r.field)[i*MAX_STRING_SIZE], ((epicsInt32*)p2)[i]); |
874 | + i, &((const char*)pfl1->u.r.field)[i*pfl1->field_size], ((epicsInt32*)p2)[i]); |
875 | return 0; |
876 | } |
877 | break; |
878 | @@ -120,7 +120,7 @@ static void testHead (const char *title, const char *typ = "") { |
879 | off = Offset; \ |
880 | (void) dbPutField(&offaddr, DBR_LONG, &off, 1); \ |
881 | pfl = db_create_read_log(pch); \ |
882 | - testOk(pfl->type == dbfl_type_rec, "original field log has type rec"); \ |
883 | + testOk(pfl->type == dbfl_type_ref, "original field log has type ref"); \ |
884 | pfl2 = dbChannelRunPostChain(pch, pfl); \ |
885 | testOk(pfl2 == pfl, "call does not drop or replace field_log"); \ |
886 | testOk(pfl->type == dbfl_type_ref, "filtered field log has type ref"); \ |
887 | diff --git a/modules/database/test/std/filters/dbndTest.c b/modules/database/test/std/filters/dbndTest.c |
888 | index e7897e2..0206865 100644 |
889 | --- a/modules/database/test/std/filters/dbndTest.c |
890 | +++ b/modules/database/test/std/filters/dbndTest.c |
891 | @@ -130,7 +130,7 @@ MAIN(dbndTest) |
892 | dbEventCtx evtctx; |
893 | int logsFree, logsFinal; |
894 | |
895 | - testPlan(77); |
896 | + testPlan(72); |
897 | |
898 | testdbPrepare(); |
899 | |
900 | @@ -171,12 +171,9 @@ MAIN(dbndTest) |
901 | "dbnd has one filter with argument in pre chain"); |
902 | testOk((ellCount(&pch->post_chain) == 0), "dbnd has no filter in post chain"); |
903 | |
904 | - /* Field logs of type ref and rec: pass any update */ |
905 | - |
906 | - testHead("Field logs of type ref and rec"); |
907 | - fl1.type = dbfl_type_rec; |
908 | - mustPassTwice(pch, &fl1, "abs field_log=rec", 0., 0); |
909 | + /* Field logs of type ref: pass any update */ |
910 | |
911 | + testHead("Field logs of type ref"); |
912 | fl1.type = dbfl_type_ref; |
913 | mustPassTwice(pch, &fl1, "abs field_log=ref", 0., 0); |
914 |
I see now that the diff is dominated by my first commit that reformats dbAccess.c. The actual refactor makes only very few changes there. If this hurts I can rebase this patch out of the current branch. How do you people treat code reformatting patches? I did this one with my standard C indent config that very closely matches the standard EPICS base style.