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!
- References:
- Hacking rsrv/camessage.c Michael Abbott
- Navigate by Date:
- Prev:
RE: EPICS protocol header Jeff Hill
- Next:
Re: [Merge] lp:~michael-abbott/epics-base/dynamic-array into lp:epics-base Jeff Hill
- Index:
2002
2003
2004
2005
2006
2007
2008
2009
<2010>
2011
2012
2013
2014
2015
2016
2017
2018
2019
2020
2021
2022
2023
2024
- Navigate by Thread:
- Prev:
RE: Hacking rsrv/camessage.c Jeff Hill
- Next:
EPICS protocol header Michael Abbott
- Index:
2002
2003
2004
2005
2006
2007
2008
2009
<2010>
2011
2012
2013
2014
2015
2016
2017
2018
2019
2020
2021
2022
2023
2024
|