Experimental Physics and
Industrial Control System

Jeong Han Lee <[email protected]> · Fri, 28 Apr 2017 09:19:02 +0200

Hi Michael,

A couple of points.

* The recGblCheckDeadband() function accepts monitor mask as
(unsigned*), which happens to be the same as (epicsUInt32*).  In my mind
though this should be explicit and the 'monitor_mask' local in
aiRecord.c should be 'unsigned'.

* Changing a local loop index from 'int i' to 'epicsInt32 i' doesn't
seem to me so useful.  Better to check if it can be made unsigned or size_t.

* In permissiveRecord.c you change some locals from 'unsigned short' to
epicsUInt16 which is fine, but you also initialize them to zero when
they were previously not initialized.  In this case it's a no-op as the
zeros are immediately replaced.  In general I think it's better to leave
locals uninitialized unless the value will be used.

gcc at least will in many cases warn about possible unconditional use of
uninitialized locals.  I like to give the compiler an opportunity to
tell me when I'm making a mistake :)

Also, I'd rather not see the type repeated four times.  So better to
make this:

epicsUInt16 val  = prec->val,
            oval = prec->oval,
            wflg = prec->wflg;
            oflg = prec->oflg;

Thanks for your comments and explanation. It is the real one that I

would like to listen from like you. Thank you.

I will re-think and re-do to create the patch file according to your

comments and suggestions.

Han

Experimental Physics and Industrial Control System

Experimental Physics and
Industrial Control System