Subject: |
Re: [Merge] lp:~michael-abbott/epics-base/dynamic-array into lp:epics-base |
From: |
Jeff Hill <[email protected]> |
To: |
[email protected] |
Date: |
Tue, 22 Jun 2010 20:55:48 -0000 |
Hi Michael,
Thanks for your help.
Please add a client side initiated regression test to acctst.c that verifies that the new feature works properly. Sorry, that acctst.c is a messy code for certain but it does serve its purpose of preventing introduction of bugs that have been seen before. This will also have the benefit of not allowing my neglect of parallel changes for these features in the new server :^). Also, running acctst might even help to test the features you have added.
changes in ca/caProto.h
> +# define CA_V412(MINOR) ((MINOR)>=12u) /* Allow zero length in
> requests. */
The following is what I have on the Bazaar branch which was based on the cvs main trunk (this is where I have always added new features in the past so that I didn't mix together patch level changes in the R3.14 branch (i.e. fixes) with feature upgrades).
# define CA_V412(MINOR) ((MINOR)>=12u) /* TCP-based search requests */
# define CA_V413(MINOR) ((MINOR)>=13u) /* channel connectivity response messages */
So we have some conflicts with the new TCP search protocol and some capabilities I have in the new server which allow the service to report the current state of the connectivity it has with its channel.
I propose the following based on the order (in time) that each upgrade might be installed into base (the TCP based search request changes were completed a long time ago):
o V412 - TCP based search requests
o V413 - dynamic payload sized subscription / get callback response message
o V414 - connectivity response messages
I am confused about the purpose of this change (presumably the old code was safer because the client code's compiler can check for access past the end of the array)?
diff --git a/src/ca/db_access.h b/src/ca/db_access.h
-epicsShareExtern const unsigned short dbr_size[LAST_BUFFER_TYPE+1];
+epicsShareExtern const unsigned short dbr_size[];
/* size for each type's value - array indexed by the DBR_ type code */
-epicsShareExtern const unsigned short dbr_value_size[LAST_BUFFER_TYPE+1];
+epicsShareExtern const unsigned short dbr_value_size[];
Changes to rssrv/camessage.c
I am a bit concerned that if for whatever reason db_get_field_and_count returns an item_count greater than requested then we could have data_size greater than payload_size which could cause the unsigned subtract to overflow here. That could cause memset to copy many (way too many) elements which would almost certainly profoundly crash the IOC. Can we afford the risk? I would add an "if (payload_size - data_size)" sanity check here. Yes, nannying but robust.
returns data_size greater
+ memset(
+ (char *) pPayload + data_size, 0, payload_size - data_size);
Otherwise, I did finally have a very close look today, and it looks good.
--
https://code.launchpad.net/~michael-abbott/epics-base/dynamic-array/+merge/26796
Your team EPICS Core Developers is requested to review the proposed merge of lp:~michael-abbott/epics-base/dynamic-array into lp:epics-base.
- Replies:
- RE: [Merge] lp:~michael-abbott/epics-base/dynamic-array intolp:epics-base Michael Abbott
- RE: [Merge] lp:~michael-abbott/epics-base/dynamic-array intolp:epics-base nick.rees
- References:
- [Merge] lp:~michael-abbott/epics-base/dynamic-array into lp:epics-base Michael Abbott
- Navigate by Date:
- Prev:
RE: Hacking rsrv/camessage.c Jeff Hill
- Next:
RE: [Merge] lp:~michael-abbott/epics-base/dynamic-array intolp:epics-base 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:
Re: [Merge] lp:~michael-abbott/epics-base/dynamic-array into lp:epics-base Michael Abbott
- Next:
RE: [Merge] lp:~michael-abbott/epics-base/dynamic-array intolp:epics-base 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
|