EPICS Base

Merge lp:~mdavidsaver/epics-base/rec-init into lp:~epics-core/epics-base/3.14

  1. rec-init
  2. Merge into 3.14
Proposed by mdavidsaver
Status: Merged
Merge reported by: Andrew Johnson
Merged at revision: not available
Proposed branch: lp:~mdavidsaver/epics-base/rec-init
Merge into: lp:~epics-core/epics-base/3.14
Diff against target: 239 lines (+56/-4)
15 files modified
src/rec/aSubRecord.c (+3/-0)
src/rec/aiRecord.c (+4/-0)
src/rec/aoRecord.c (+5/-0)
src/rec/biRecord.c (+3/-0)
src/rec/boRecord.c (+5/-0)
src/rec/calcoutRecord.c (+5/-0)
src/rec/longinRecord.c (+3/-0)
src/rec/longoutRecord.c (+3/-0)
src/rec/mbbiDirectRecord.c (+3/-0)
src/rec/mbbiRecord.c (+3/-0)
src/rec/mbboDirectRecord.c (+4/-0)
src/rec/mbboRecord.c (+4/-0)
src/rec/stringinRecord.c (+4/-2)
src/rec/stringoutRecord.c (+4/-2)
src/rec/subRecord.c (+3/-0)
To merge this branch: bzr merge lp:~mdavidsaver/epics-base/rec-init
Reviewer Review Type Date Requested Status
Andrew Johnson Approve
EPICS Core Developers Pending
Review via email: mp+22819@code.launchpad.net

Commit message

Correctly initialize various fields used to detect changes in other fields (MLST, OVAL, LALM, ...)

Currently this are zero. This is a problem if the fields they are compared against are initialized (in init_record) to a non-zero value.

Description of the change

Correctly initialize various fields used to detect changes in other fields (MLST, OVAL, LALM, ...)

Currently this are zero. This is a problem if the fields they are compared against are initialized (in init_record) to a non-zero value.

Fields effected

in many recordtypes

MLST -> VAL
ALST -> VAL
LALM -> VAL
ORAW -> RVAL
ORBV -> RBV

in calcout
POVL -> OVAL

in aSub

ONAM -> SNAM

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

Your stringin/stringout changes contain this:

    strncpy(prec->oval,prec->val,sizeof(prec->val));

Please use sizeof(prec->oval) instead, since that's the destination of the string copy. It would be a bug if the sizes of the two fields are ever different, but this is more robust if that does happen.

Otherwise I think this looks good.

review: Needs Fixing
lp:~mdavidsaver/epics-base/rec-init updated
12049. By mdavidsaver

Ensure the string VAL and OVAL have the same storage length

There won't be any problems with overflow as long as dbPut
behaves correctly when setting VAL.

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

> Your stringin/stringout changes contain this:
>
> strncpy(prec->oval,prec->val,sizeof(prec->val));
>
> Please use sizeof(prec->oval) instead, since that's the destination of the
> string copy. It would be a bug if the sizes of the two fields are ever
> different, but this is more robust if that does happen.

Add a STATIC_ASSERT of the two field sizes since they should never be different. Also for aSub.

Also since val is set by dbPut it will always be null terminated (see dbConvert.c). The aSub recordtype was assuming this already.

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

Good solution.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/rec/aSubRecord.c'
2--- src/rec/aSubRecord.c 2009-07-09 15:27:43 +0000
3+++ src/rec/aSubRecord.c 2010-04-06 18:51:34 +0000
4@@ -210,6 +210,9 @@
5 return S_db_BadSub;
6 }
7 }
8+ STATIC_ASSERT(sizeof(prec->onam)==sizeof(prec->snam));
9+ strcpy(prec->onam, prec->snam);
10+ prec->oval = prec->val;
11 return 0;
12 }
13
14
15=== modified file 'src/rec/aiRecord.c'
16--- src/rec/aiRecord.c 2009-04-03 14:40:13 +0000
17+++ src/rec/aiRecord.c 2010-04-06 18:51:34 +0000
18@@ -145,6 +145,10 @@
19 }
20 return (status);
21 }
22+ prec->mlst = prec->val;
23+ prec->alst = prec->val;
24+ prec->lalm = prec->val;
25+ prec->oraw = prec->rval;
26 return(0);
27 }
28
29
30
31=== modified file 'src/rec/aoRecord.c'
32--- src/rec/aoRecord.c 2009-07-08 18:14:11 +0000
33+++ src/rec/aoRecord.c 2010-04-06 18:51:34 +0000
34@@ -166,6 +166,11 @@
35 }
36 }
37 prec->oval = prec->pval = prec->val;
38+ prec->mlst = prec->val;
39+ prec->alst = prec->val;
40+ prec->lalm = prec->val;
41+ prec->oraw = prec->rval;
42+ prec->orbv = prec->rbv;
43 return(0);
44 }
45
46
47
48=== modified file 'src/rec/biRecord.c'
49--- src/rec/biRecord.c 2009-04-02 21:41:45 +0000
50+++ src/rec/biRecord.c 2010-04-06 18:51:34 +0000
51@@ -110,6 +110,9 @@
52 if( pdset->init_record ) {
53 if((status=(*pdset->init_record)(prec))) return(status);
54 }
55+ prec->mlst = prec->val;
56+ prec->lalm = prec->val;
57+ prec->oraw = prec->rval;
58 return(0);
59 }
60
61
62
63=== modified file 'src/rec/boRecord.c'
64--- src/rec/boRecord.c 2010-03-24 18:21:38 +0000
65+++ src/rec/boRecord.c 2010-04-06 18:51:34 +0000
66@@ -177,6 +177,11 @@
67 if(prec->val==0) prec->rval = 0;
68 else prec->rval = prec->mask;
69 } else prec->rval = (epicsUInt32)prec->val;
70+
71+ prec->mlst = prec->val;
72+ prec->lalm = prec->val;
73+ prec->oraw = prec->rval;
74+ prec->orbv = prec->rbv;
75 return(status);
76 }
77
78
79
80=== modified file 'src/rec/calcoutRecord.c'
81--- src/rec/calcoutRecord.c 2009-07-09 15:27:43 +0000
82+++ src/rec/calcoutRecord.c 2010-04-06 18:51:34 +0000
83@@ -198,6 +198,11 @@
84 prpvt->cbScheduled = 0;
85
86 if (pcalcoutDSET->init_record) pcalcoutDSET->init_record(prec);
87+ prec->pval = prec->val;
88+ prec->mlst = prec->val;
89+ prec->alst = prec->val;
90+ prec->lalm = prec->val;
91+ prec->povl = prec->oval;
92 return 0;
93 }
94
95
96
97=== modified file 'src/rec/longinRecord.c'
98--- src/rec/longinRecord.c 2009-07-08 18:14:11 +0000
99+++ src/rec/longinRecord.c 2010-04-06 18:51:34 +0000
100@@ -121,6 +121,9 @@
101 if( pdset->init_record ) {
102 if((status=(*pdset->init_record)(prec))) return(status);
103 }
104+ prec->mlst = prec->val;
105+ prec->alst = prec->val;
106+ prec->lalm = prec->val;
107 return(0);
108 }
109
110
111
112=== modified file 'src/rec/longoutRecord.c'
113--- src/rec/longoutRecord.c 2009-07-08 18:14:11 +0000
114+++ src/rec/longoutRecord.c 2010-04-06 18:51:34 +0000
115@@ -117,6 +117,9 @@
116 if( pdset->init_record ) {
117 if((status=(*pdset->init_record)(prec))) return(status);
118 }
119+ prec->mlst = prec->val;
120+ prec->alst = prec->val;
121+ prec->lalm = prec->val;
122 return(0);
123 }
124
125
126
127=== modified file 'src/rec/mbbiDirectRecord.c'
128--- src/rec/mbbiDirectRecord.c 2009-07-08 18:14:11 +0000
129+++ src/rec/mbbiDirectRecord.c 2010-04-06 18:51:34 +0000
130@@ -158,6 +158,9 @@
131 if((status=(*pdset->init_record)(prec))) return(status);
132 refresh_bits(prec, 0);
133 }
134+ prec->mlst = prec->val;
135+ prec->lalm = prec->val;
136+ prec->oraw = prec->rval;
137 return(0);
138 }
139
140
141=== modified file 'src/rec/mbbiRecord.c'
142--- src/rec/mbbiRecord.c 2009-04-02 21:41:45 +0000
143+++ src/rec/mbbiRecord.c 2010-04-06 18:51:34 +0000
144@@ -135,6 +135,9 @@
145 if((status=(*pdset->init_record)(prec))) return(status);
146 }
147 init_common(prec);
148+ prec->mlst = prec->val;
149+ prec->lalm = prec->val;
150+ prec->oraw = prec->rval;
151 return(0);
152 }
153
154
155
156=== modified file 'src/rec/mbboDirectRecord.c'
157--- src/rec/mbboDirectRecord.c 2009-07-08 18:14:11 +0000
158+++ src/rec/mbboDirectRecord.c 2010-04-06 18:51:34 +0000
159@@ -141,6 +141,10 @@
160 prec->udf = FALSE;
161 } else if (status == 2) status = 0;
162 }
163+ prec->mlst = prec->val;
164+ prec->lalm = prec->val;
165+ prec->oraw = prec->rval;
166+ prec->orbv = prec->rbv;
167 return(status);
168 }
169
170
171=== modified file 'src/rec/mbboRecord.c'
172--- src/rec/mbboRecord.c 2009-07-08 18:14:11 +0000
173+++ src/rec/mbboRecord.c 2010-04-06 18:51:34 +0000
174@@ -184,6 +184,10 @@
175 init_common(prec);
176 /* convert val to rval */
177 convert(prec);
178+ prec->mlst = prec->val;
179+ prec->lalm = prec->val;
180+ prec->oraw = prec->rval;
181+ prec->orbv = prec->rbv;
182 return(0);
183 }
184
185
186
187=== modified file 'src/rec/stringinRecord.c'
188--- src/rec/stringinRecord.c 2009-07-08 18:14:11 +0000
189+++ src/rec/stringinRecord.c 2010-04-06 18:51:34 +0000
190@@ -119,6 +119,8 @@
191 if( pdset->init_record ) {
192 if((status=(*pdset->init_record)(prec))) return(status);
193 }
194+ STATIC_ASSERT(sizeof(prec->oval)==sizeof(prec->val));
195+ strcpy(prec->oval,prec->val);
196 return(0);
197 }
198
199
200@@ -157,9 +159,9 @@
201 unsigned short monitor_mask;
202
203 monitor_mask = recGblResetAlarms(prec);
204- if(strncmp(prec->oval,prec->val,sizeof(prec->val))) {
205+ if(strcmp(prec->oval,prec->val)) {
206 monitor_mask |= DBE_VALUE|DBE_LOG;
207- strncpy(prec->oval,prec->val,sizeof(prec->val));
208+ strcpy(prec->oval,prec->val);
209 }
210 if (prec->mpst == stringinPOST_Always)
211 monitor_mask |= DBE_VALUE;
212
213=== modified file 'src/rec/stringoutRecord.c'
214--- src/rec/stringoutRecord.c 2010-03-01 19:19:43 +0000
215+++ src/rec/stringoutRecord.c 2010-04-06 18:51:34 +0000
216@@ -121,6 +121,8 @@
217 if( pdset->init_record ) {
218 if((status=(*pdset->init_record)(prec))) return(status);
219 }
220+ STATIC_ASSERT(sizeof(prec->oval)==sizeof(prec->val));
221+ strcpy(prec->oval,prec->val);
222 return(0);
223 }
224
225
226@@ -185,9 +187,9 @@
227 unsigned short monitor_mask;
228
229 monitor_mask = recGblResetAlarms(prec);
230- if(strncmp(prec->oval,prec->val,sizeof(prec->val))) {
231+ if(strcmp(prec->oval,prec->val)) {
232 monitor_mask |= DBE_VALUE|DBE_LOG;
233- strncpy(prec->oval,prec->val,sizeof(prec->val));
234+ strcpy(prec->oval,prec->val);
235 }
236 if (prec->mpst == stringoutPOST_Always)
237 monitor_mask |= DBE_VALUE;
238
239=== modified file 'src/rec/subRecord.c'
240--- src/rec/subRecord.c 2009-07-28 21:11:02 +0000
241+++ src/rec/subRecord.c 2010-04-06 18:51:34 +0000
242@@ -127,6 +127,9 @@
243 recGblRecordError(S_db_BadSub, (void *)prec, "recSub(init_record)");
244 return S_db_BadSub;
245 }
246+ prec->mlst = prec->val;
247+ prec->alst = prec->val;
248+ prec->lalm = prec->val;
249 return 0;
250 }
251

Subscribers

People subscribed via source and target branches