Argonne National Laboratory

Experimental Physics and
Industrial Control System

2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  <2017 Index 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  <2017
<== Date ==> <== Thread ==>

Subject: Re: Patch file for Records which use consistent epics data types
From: Jeong Han Lee <citadel.lee@gmail.com>
To: Michael Davidsaver <mdavidsaver@gmail.com>
Cc: core-talk@aps.anl.gov
Date: 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

References:
Patch file for Records which use consistent epics data types Jeong Han Lee
Re: Patch file for Records which use consistent epics data types Michael Davidsaver

Navigate by Date:
Prev: Re: Patch file for Records which use consistent epics data types Michael Davidsaver
Next: Jenkins build is back to normal : epics-base-3.16-win64-test #50 APS Jenkins
Index: 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  <2017
Navigate by Thread:
Prev: Re: Patch file for Records which use consistent epics data types Michael Davidsaver
Next: Build failed in Jenkins: epics-base-3.16-mac-test #78 APS Jenkins
Index: 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  <2017
ANJ, 28 Apr 2017 Valid HTML 4.01! · Home · News · About · Base · Modules · Extensions · Distributions · Download ·
· EPICS V4 · IRMIS · Talk · Bugs · Documents · Links · Licensing ·