Re: sequencer feature request

[Dirk Zimoch <dirk.zimoch@psi.ch>]

Hi Mr. Small IOC,

What is the stack size of your small IOC? Is allocating 200 bytes on the stack 
OK? Apart from that, I cannot find any impact on the rest of the sequencer code.
There are other occurrences of MAX_STRING_SIZE but they are probably OK:

seq/seq_main.c line 98:
#define      SCRATCH_SIZE    (MAX_MACROS*(MAX_STRING_SIZE+1)*12)
But SCRATCH_SIZE is never used. It should be deleted.

seq/seq_main.c line 563:
} typeMap[] = {
...
	{
	"string",	 pvTypeSTRING,	pvTypeTIME_STRING,
	MAX_STRING_SIZE, OFFSET(pvTimeString, value[0])
	},

snc/phase2.c line 153:
    printf("#define MAX_STRING_SIZE 40\n");

snc/phase2.c line 400:
			printf("[MAX_STRING_SIZE]");

These occurrences are related to string variables and seem to be OK for the moment.


I also looked for other hard coded limits and found:

seq/seq_mac.c line 47:
	char		name[50], *pValue, *pTmp;

This limits the length of a macro name (not its value) to 49 chars. Probably not 
a serious problem but still quite unnecessary. A dynamic solution would be to 
temporarily allocate memory of the size of the substitution sting.
The number of macros is limited to 50 by
seq/seqPvt.h line 226:
#define    MAX_MACROS      50
This can be made dynamic by replacing the table with a linked list of macros.

In principle, it is not necessary to use a macro table. Whenever a macro is 
referenced, it can be looked up in the substitution string. The performance loss 
should be negligible. Macros are only looked up during initialization.
Dirk


Eric Norum wrote:
> MACRO_STR_LEN is used only in that one function -- here's my suggestion
>
> /*
>  * Evaluate channel names by macro substitution.
>  */
> #define     MACRO_STR_LEN   200
> LOCAL void seqChanNameEval(pSP)
> SPROG       *pSP;
> {
>     CHAN        *pDB;
>     int     i;
>
>     pDB = pSP->pChan;
>     for (i = 0; i < pSP->numChans; i++, pDB++)
>     {
>         char cbuf[MACRO_STR_LEN+1];
>         seqMacEval(pDB->dbAsName, cbuf, MACRO_STR_LEN, pSP->pMacros);
>         pDB->dbName = epicsStrDup(cbuf);
> #ifdef  DEBUG
>         printf("seqChanNameEval: \"%s\" evaluated to \"%s\"\n",
>             pDB->dbAsName, pDB->dbName);
> #endif  /*DEBUG*/
>     }
> }
>
> On Sep 9, 2009, at 12:54 PM, Chestnut, Ronald P. wrote:
>
> > Mr. Expedient here.
> >
> > With any luck, all the code in the sequencer uses MACRO_STR_LEN, and a 
> > change to something larger but static (perhaps 61 or 100?) might just 
> > work after a recompile. Doing something dynamic requires actual 
> > thinking and more extensive testing. How about an initial static bump 
> > with a subsequent dynamic solution?
> > Ron Chestnut
> >
> >
> > -----Original Message-----
> > From: tech-talk-bounces@aps.anl.gov 
> > [mailto:tech-talk-bounces@aps.anl.gov] On Behalf Of Eric Norum
> > Sent: Wednesday, September 09, 2009 6:15 AM
> > To: tech-talk@aps.anl.gov Techtalk
> > Subject: Re: sequencer feature request
> >
> > Mr. Small IOC here.
> > Rather than allocating the maximum-sized space for every single name,
> > why not expand the name into a 'big-enough' buffer and then allocate
> > only the required size and copy the result?
> >
> > On Sep 9, 2009, at 5:13 AM, Dirk Zimoch wrote:
> >
> > > I found several places in the code where the sequencer uses
> > > MAX_STRING_SIZE+1 for string lengths of PV names and macros. In
> > > EPICS base MAX_STRING_SIZE is defined as 40. For example in
> > > seq_main.c:
> > >
> > > /*
> > > * Evaluate channel names by macro substitution.
> > > */
> > > #define        MACRO_STR_LEN    (MAX_STRING_SIZE+1)
> > > LOCAL void seqChanNameEval(pSP)
> > > SPROG        *pSP;
> > > {
> > >     CHAN        *pDB;
> > >     int        i;
> > >
> > >     pDB = pSP->pChan;
> > >     for (i = 0; i < pSP->numChans; i++, pDB++)
> > >     {
> > >         pDB->dbName = calloc(1, MACRO_STR_LEN);
> > >         seqMacEval(pDB->dbAsName, pDB->dbName, MACRO_STR_LEN, 
> > > pSP->pMacros);
> > > #ifdef    DEBUG
> > >         printf("seqChanNameEval: \"%s\" evaluated to \"%s\"\n",
> > >             pDB->dbAsName, pDB->dbName);
> > > #endif    /*DEBUG*/
> > >     }
> > > }
> > >
> > > It might help to replace those uses of MAX_STRING_SIZE+1 with
> > > PVNAME_STRINGSZ.
> > >
> > > Dirk
> > >
> > >
> > >
> > > Dirk Zimoch wrote:
> > > > I tested the sequencer with a long name and it seems that it
> > > > truncates the PV name to 41 chars.
> > > > A seqChanShow shows the difference between the name used in assign
> > > > and the actual name used in CA:
> > > > Channel name: "DZ:45678901234567890123456789012345678901"
> > > > Unexpanded (assigned) name: "DZ:
> > > > 456789012345678901234567890123456789012345678901234567890"
> > > > Dirk
> > > > Mark Rivers wrote:
> > > > > EPICS base defines the maximum length of the PV name to be
> > > > > PVNAME_STRINGSZ, which is currently 61 including the nil terminator.
> > > > > What length limitation does the sequencer impose?  Is it more
> > > > > restrictive than this?
> > > > > Mark
> > > > >
> > > > > ________________________________
> > > > >
> > > > > From: tech-talk-bounces@aps.anl.gov on behalf of Patrick Thomas
> > > > > Sent: Thu 9/3/2009 6:29 PM
> > > > > To: tech-talk@aps.anl.gov
> > > > > Subject: sequencer feature request
> > > > >
> > > > >
> > > > >
> > > > > Hi,
> > > > >
> > > > > I was wondering if I could ask for a feature request for the next
> > > > > version of the sequencer? Is there any chance that it would be
> > > > > possible
> > > > > to use unlimited length record names in assign to statements?
> > > > >
> > > > > Thank you,
> > > > > Patrick
> > > --Dr. Dirk Zimoch
> > > Paul Scherrer Institut, WBGB/006
> > > 5232 Villigen PSI, Switzerland
> > > Phone +41 56 310 5182
> > --Eric Norum <norume@aps.anl.gov>
> > Advanced Photon Source
> > Argonne National Laboratory
> > (630) 252-4793
> --Eric Norum <norume@aps.anl.gov>
> Advanced Photon Source
> Argonne National Laboratory
> (630) 252-4793
--
Dr. Dirk Zimoch
Paul Scherrer Institut, WBGB/006
5232 Villigen PSI, Switzerland
Phone +41 56 310 5182