Experimental Physics and
Industrial Control System

"Jeff Hill" <[email protected]> · Tue, 22 Jun 2010 10:23:50 -0600

PS: As I recall the code _does_ on scalar strings check the string length
and send less (8 byte aligned) characters if the string is less than
MAX_STRING_SIZE bytes. This protocol compression would be useful probably
only on slow machines or slow links. Maybe only with IP over slow RS232
links, or over slow modem links. There was at one time a use of EPICS on a
scada system implemented with modems.

So, from that perspective, the code is already checking the string length of
scalars so the additional overhead may not be 100% classified as nannying.
Nevertheless, detection of an interface violation in db_get_field would be
disconcerting so I added a big fat message. I don?t know if anyone has
actually seen this message - that would provide some additional confidence
that a real issue exists?

> +                    errlogPrintf (
> +                        "caserver: read_reply: detected DBR_STRING w/o
> nill termination "
> +                        "in response from db_get_field, pPayload =
> \"%s\"\n",
> +                        pStr );

Jeff
______________________________________________________
Jeffrey O. Hill           Email        [email protected]
LANL MS H820              Voice        505 665 1831
Los Alamos NM 87545 USA   FAX          505 665 5107

Message content: TSPA

> -----Original Message-----
> From: Michael Abbott [mailto:[email protected]]
> Sent: Wednesday, June 02, 2010 12:31 AM
> To: EPICS core; Jeff Hill
> Subject: Hacking rsrv/camessage.c
> 
> I'm in the middle of reworking read_reply() (rsrv/camessage.c) to
> implement support for autoresizing arrays (I hope to post my patches
> later
> this week), and I've encountered a curious bit of special case code that
> I'm about to erase ... so thought I'd better ask about it.
> 
> I'm puzzled by this commit (I've edited out all the rest of the diff, as
> it doesn't affect my changes):
> 
> 
> revno: 10992
> committer: Jeff Hill <[email protected]>
> branch nick: B3.14
> timestamp: Fri 2007-09-07 17:43:52 +0000
> message:
>   fix for mantis entry 300:
>   'assert (size <= ntohs ( pMsg->m_postsize ))' failed in ..caserverio.c
> line 344
> diff:
> === modified file 'src/rsrv/camessage.c'
> --- src/rsrv/camessage.c        2007-08-17 22:31:11 +0000
> +++ src/rsrv/camessage.c        2007-09-07 17:43:52 +0000
> @@ -586,16 +598,25 @@
>              */
>              if ( pevext->msg.m_dataType == DBR_STRING
>                  && pevext->msg.m_count == 1 ) {
> -                /* add 1 so that the string terminator will be shipped
> */
> -                strcnt = strlen ( (char *) pPayload ) + 1;
> -                msgSize = strcnt;
> +                char * pStr = (char *) pPayload;
> +                size_t strcnt = strlen ( pStr );
> +                if ( strcnt < payloadSize ) {
> +                    payloadSize = ( ca_uint32_t ) ( strcnt + 1u );
> +                }
> +                else {
> +                    pStr[payloadSize-1] = '\0';
> +                    errlogPrintf (
> +                        "caserver: read_reply: detected DBR_STRING w/o
> nill termination "
> +                        "in response from db_get_field, pPayload =
> \"%s\"\n",
> +                        pStr );
> +                }
>              }
>          }
>          else {
> -            memset ( pPayload, 0, msgSize );
> +            memset ( pPayload, 0, payloadSize );
>              cas_set_header_cid ( pClient, cacStatus );
>             }
> -        cas_commit_msg ( pClient, msgSize );
> +        cas_commit_msg ( pClient, payloadSize );
>      }
> 
>      /*
> 
> 
> 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?
> 
> My patches are going to drop this block of code altogether (so really my
> inquiries ought to also address the parent patches), but thought I'd
> better highlight this separately!

Experimental Physics and Industrial Control System

Experimental Physics and
Industrial Control System