Experimental Physics and Industrial Control System
Hi Michael,
> 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.
Ooh, careful...
> 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?
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.
You also have the Mantis bug number that this was fixing, so you should read the thread at https://bugs.launchpad.net/epics-base/+bug/mantis-300 to follow that discussion, in case it helps.
Jeff will probably not be able to read email until he gets back to LANL next week.
- Andrew
PS: Could you re-post your patches by publishing a Bazaar branch and proposing a merge? It's going to have to happen eventually, and it's easier to look at them that way IMHO...
- Replies:
- RE: Hacking rsrv/camessage.c michael.abbott
- RE: Hacking rsrv/camessage.c Jeff Hill
- References:
- Hacking rsrv/camessage.c Michael Abbott
- Navigate by Date:
- Prev:
[PATCH 0/4] Adding dynamic arrays to EPICS Channel Access Michael Abbott
- Next:
RE: Hacking rsrv/camessage.c 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
- Navigate by Thread:
- Prev:
Hacking rsrv/camessage.c Michael Abbott
- Next:
RE: Hacking rsrv/camessage.c 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