Experimental Physics and
Industrial Control System

<[email protected]> · Fri, 4 Jun 2010 10:56:06 +0100

I've realised that we're sharing a misunderstanding, at least I *think that's the case!

From: Abbott, Michael (DLSLtd,RAL,DIA) 
> > >              if ( pevext->msg.m_dataType == DBR_STRING 
> > >                  && pevext->msg.m_count == 1 ) {
> > > 
> > > I'm quite baffled.  Is there any reason not to *always* ship the 
> > > entire 40 characters of the EPICS string as a matter of course?  
> > > Or is this a fine grained optimisation to avoid shipping an 
> > > extra average 20 bytes?
> > 
> > String fields on the database side are not limited to 40 
> > characters long; the db_access API can't zero-terminate the 
> > source buffer.  Make sure that this code isn't protecting 
> > against that kind of condition, which is easiest to test 
> > using a record with a name longer than 40 characters and 
> > accessing its .NAME field.

I think our shared misunderstanding is in the role of DBR_STRING in our conversation.  I presume that your new pv$ strings are actually acquired through dbGetField(... DBR_CHAR, ..., &MAX_STRING_LEN, ...) -- in fact, they *must* be!

In particular, this means that the bit of code we're talking about is bogus ... except, perhaps, for the lingering issue of null termination, of which more presently.

My code seems to work ok with an overlength .NAME field: if a request is made for pv.NAME I get the name truncated to 39 characters (plus null termination), and a request for pv.NAME$ as DBR_CHAR returns the sensible string.  Interestingly I see some zero padding at the end of the char array, but presumably that's simply because dbGetField returns the entire maximum possible name length and doesn't update the request count.  Maybe a small fix worth doing...

So most of my remarks below are, I believe, me panicking!  Sorry about that.

However, I do have a bug: my IOC crashes if the name is too long, but the 3.14.11 detects the error.  I'm not sure how my code can be responsible, but I'll have to check.

> Ouch. OUCH.
> 
> So I take it that such long string handling only applies to 
> single strings, not to arrays of strings?  Is there a limit 
> on the length of such strings?
> 
> Unfortunately I see two nasty problems here, unless I'm 
> missing something.
> 
> Firstly, the call to the client database goes through 
> dbGetField() which provides no information about the actual 
> buffer size, merely about the number of elements -- so I 
> *have* to conclude that there's a hard-wired implicit limit 
> on the string length somewhere in this API, and presumably 
> the dbGetField() call must have to guarantee to null 
> terminate the string it returns!
> 
> So let me see if I've got this right:  
> 
> dbGetField(dbaddr, DBR_STRING, buffer, popt, pn, pfl) copies 
> up to MAX_STRING_LEN (some hard-wired constant I've not yet 
> discovered) NULL terminated bytes to buffer[] if and ONLY IF 
> *popt == 0 and *pn == 1.  If *pn > 1 then standard EPICS 
> strings are copied,

Ok, all this has got to be bogus.  If a request is made through dbGetField for a DBR_STRING value, I feel confident that the call must restrict itself to the 40 characters it's been given.  What happens if the string is *exactly* 40 characters (no room for a null termination) is an amusing problem for the client ... at least, that's my first thought.

I'm inclined to take the view that it's the responsibility of dbGetField to do the null termination, which it seems to do for the strings I'm looking at, and certainly there isn't reliable DBR_*_STRING termination code anywhere.  I won't worry about this.

>  if *popt != 0 only predefined attributes 
> are copied.  Is this right?  Is this specified anywhere?
> 
> I'm looking at the EPICS Application Developer's Guide, and I 
> see some odd things.  The guide implies that the value is 
> always copied, but the code for db_get_field would generate a 
> stack overwrite if that was the case.  I see reference to a 
> dbBufferSize routine in the documentation, but this is not 
> used anywhere in EPICS base, so I imagine it's bogus. 

Unfortunately this bit above remains valid.  Haven't dug into the corresponding code yet, but would guess that the documentation is remiss.  Maybe dbBufferSize is designed for external consumption?  Otherwise it should be dropped!

> I can 
> find no reference to this horrible long string handling you mention.
> 
> Secondly, I can't see where enough room is allocated in the 
> buffer passed to db_get_field() from read_field().  As far as 
> I can tell, the buffer size allocation is computed by 
> dbr_size_n() which, unless I'm hugely mistaken, allocates 40 
> bytes for a string.
> 
> My guess is that we get away with this (most of the time) 
> because the buffer we use normally has plenty of room...  
> What am I missing?  This would seem a particularly evil 
> Heisenbug if I'm reading this right.

Ok, I think this above *all* wrong, just ignore it.

-- 
This e-mail and any attachments may contain confidential, copyright and or privileged material, and are for the use of the intended addressee only. If you are not the intended addressee or an authorised recipient of the addressee please notify us of receipt by returning the e-mail and do not use, copy, retain, distribute or disclose the information in or attached to the e-mail.
Any opinions expressed within this e-mail are those of the individual and not necessarily of Diamond Light Source Ltd. 
Diamond Light Source Ltd. cannot guarantee that this e-mail or any attachments are free from viruses and we cannot accept liability for any damage which you may sustain as a result of software viruses which may be transmitted in or with the message.
Diamond Light Source Limited (company no. 4375679). Registered in England and Wales with its registered office at Diamond House, Harwell Science and Innovation Campus, Didcot, Oxfordshire, OX11 0DE, United Kingdom

Experimental Physics and Industrial Control System

Experimental Physics and
Industrial Control System