From 0e3e1b3ee142cc9b0591135a43710ed86af0288b Mon Sep 17 00:00:00 2001
From: Jonathan Hanks <jonathan.hanks@ligo.org>
Date: Fri, 15 Apr 2022 15:58:24 -0700
Subject: [PATCH] EPICS Sequencer - Rework BURT parseLine in terms of fixed
vectors and strings.
* Remove the possibility to have variable string lengths passed in.
* Work with an out parameter that knows its size and can assert it to the caller.
---
src/epics/seq/burt_file.hh | 214 ++++++++++---------
src/epics/seq/fixed_size_string.hh | 13 ++
src/epics/seq/main.cc | 64 +++---
src/epics/seq/test/test_burtfile.cc | 28 +--
src/epics/seq/test/test_fixed_size_string.cc | 14 ++
5 files changed, 175 insertions(+), 158 deletions(-)
diff --git a/src/epics/seq/burt_file.hh b/src/epics/seq/burt_file.hh
index 5cf6b8d92..f28ba6d06 100644
--- a/src/epics/seq/burt_file.hh
+++ b/src/epics/seq/burt_file.hh
@@ -6,8 +6,15 @@
#define DAQD_TRUNK_BURT_FILE_HH
#include "fixed_size_string.hh"
+#include "fixed_size_vector.hh"
-/**
+namespace BURT
+{
+
+ using parsed_word = embedded::fixed_string< 128 >;
+ using parsed_line = embedded::fixed_size_vector< parsed_word, 4 >;
+
+ /**
* @brief Encode a string for output into a BURT file.
* @note BURT encoding of strings does not escape quotes,
* and lists empty strings as \0 (backslash and 0)
@@ -16,126 +23,127 @@
* @tparam DEST_SIZE Must be at least 2 greater than SRC_SIZE
* @param src Input string to encode.
* @param dest Destination for an encoded version of src.
- */
-template < std::size_t SRC_SIZE, std::size_t DEST_SIZE >
-void
-encodeBURTString( const embedded::fixed_string< SRC_SIZE >& src,
- embedded::fixed_string< DEST_SIZE >& dest )
-{
- static_assert( DEST_SIZE >= ( SRC_SIZE + 2 ),
- "The destination buffer must be at last bytes larger than "
- "the source to account for quoting" );
- static_assert( DEST_SIZE >= 2,
- "The destination buffer must be long enough to burt encode "
- "a NULL string" );
-
- if ( src.empty( ) )
+ */
+ template < std::size_t SRC_SIZE, std::size_t DEST_SIZE >
+ void
+ encodeBURTString( const embedded::fixed_string< SRC_SIZE >& src,
+ embedded::fixed_string< DEST_SIZE >& dest )
{
- dest = "\\0";
- return;
- }
+ static_assert(
+ DEST_SIZE >= ( SRC_SIZE + 2 ),
+ "The destination buffer must be at last bytes larger than "
+ "the source to account for quoting" );
+ static_assert(
+ DEST_SIZE >= 2,
+ "The destination buffer must be long enough to burt encode "
+ "a NULL string" );
- bool expand = false;
+ if ( src.empty( ) )
+ {
+ dest = "\\0";
+ return;
+ }
- for ( char ch : src )
- {
- if ( ch == ' ' || ch == '\t' || ch == '\n' || ch == '"' )
+ bool expand = false;
+
+ for ( char ch : src )
{
- expand = true;
- break;
+ if ( ch == ' ' || ch == '\t' || ch == '\n' || ch == '"' )
+ {
+ expand = true;
+ break;
+ }
+ }
+ // we already bounds check this above
+ if ( expand )
+ {
+ dest.printf( "\"%s\"", src.c_str( ) );
+ }
+ else
+ {
+ dest = src;
}
}
- // we already bounds check this above
- if ( expand )
- {
- dest.printf( "\"%s\"", src.c_str( ) );
- }
- else
- {
- dest = src;
- }
-}
-/// Common routine to parse lines read from BURT/SDF files..
-/// @param[in] *s Pointer to line of chars to parse.
-/// @param[in] bufSize Size in characters of each output buffer str1 thru str6
-/// @param[out] str1 thru str6 String pointers to return individual words from line.
-/// @return wc Number of words in the line.
-static inline
-int parseLine(const char *s, int bufSize, char *str1, char *str2, char *str3, char *str4, char *str5,char *str6)
-{
- int wc = 0;
- int ii = 0;
- int cursize = 0;
- int lastwasspace = 1;
+ /// Common routine to parse lines read from BURT/SDF files..
+ /// @param[in] *s Pointer to line of chars to parse.
+ /// @param[out] Output containing all the identified words in the line
+ /// @return wc Number of words in the line.
+ template <std::size_t N_WORDS, std::size_t WORD_LEN>
+ static inline int
+ parseLine( const char* s,
+ embedded::fixed_size_vector<
+ embedded::fixed_string<WORD_LEN>,
+ N_WORDS>& out )
+ {
+ int lastwasspace = 1;
- const char *lastquote = nullptr;
- const char *qch = nullptr;
- char *out[7];
+ const char* lastquote = nullptr;
+ const char* qch = nullptr;
- out[0]=str1;
- out[1]=str2;
- out[2]=str3;
- out[3]=str4;
- out[4]=str5;
- out[5]=str6;
- out[6]=nullptr;
- for (ii = 0; out[ii]; ++ii) { *(out[ii]) = '\0'; }
+ out.clear();
+ embedded::fixed_string<WORD_LEN> cur_word{};
- while (*s != 0 && *s != '\n') {
- if (*s == ' ' || *s == '\t') {
- // force the null termination of the output buffer
- out[wc][bufSize-1] = '\0';
- if (!lastwasspace) {
- ++wc;
- // check for the end of the output buffers
- if (!out[wc]) break;
+ while ( *s != 0 && *s != '\n' && out.capacity() > out.size() )
+ {
+ if ( *s == ' ' || *s == '\t' )
+ {
+ if ( !lastwasspace )
+ {
+ out.push_back( cur_word );
+ cur_word.clear();
+ }
+ lastwasspace = 1;
}
- lastwasspace = 1;
- cursize = 0;
- } else if (*s != '"' || !lastwasspace) {
- // regular character
- if (cursize < bufSize) {
- out[wc][cursize] = *s;
- cursize++;
+ else if ( *s != '"' || !lastwasspace )
+ {
+ cur_word += *s;
+ lastwasspace = 0;
}
- lastwasspace = 0;
- } else {
- // quote
- // burt does not escape quotes, you have to look for the last quote and just take it.
- lastquote = nullptr;
- qch = s+1;
+ else
+ {
+ // quote
+ // burt does not escape quotes, you have to look for the last quote and just take it.
+ lastquote = nullptr;
+ qch = s + 1;
- while (*qch && *qch !='\n') {
- if (*qch == '"') lastquote = qch;
- ++qch;
- }
- if (!lastquote) lastquote = qch;
- ++s;
- // copy from (s,qch) then set lastwasspace
- while (s < lastquote) {
- if (cursize < bufSize) {
- out[wc][cursize] = *s;
- cursize++;
+ while ( *qch && *qch != '\n' )
+ {
+ if ( *qch == '"' )
+ lastquote = qch;
+ ++qch;
}
+ if ( !lastquote )
+ lastquote = qch;
++s;
+ // copy from (s,qch) then set lastwasspace
+ while ( s < lastquote )
+ {
+ cur_word += *s;
+ ++s;
+ }
+ out.push_back( cur_word );
+ cur_word.clear();
+ lastwasspace = 1;
+ if ( !( *s ) || *s == '\n' || out.size() == out.capacity() )
+ break;
}
- out[wc][bufSize-1] = '\0';
- ++wc;
- cursize = 0;
- lastwasspace = 1;
- if (!(*s) || *s == '\n' || !out[wc]) break;
+ ++s;
}
- ++s;
- }
- if (out[wc] && out[wc][0]) {
- out[wc][bufSize-1]='\0';
- ++wc;
- }
- for (ii = 0; out[ii]; ++ii) {
- if (strcmp(out[ii], "\\0") == 0) strcpy(out[ii], "");
+ if ( !cur_word.empty( ) )
+ {
+ out.push_back( cur_word );
+ }
+ for ( auto& word:out )
+ {
+ if ( word == "\\0" )
+ {
+ word = "";
+ }
+ }
+ return out.size();
}
- return wc;
+
}
#endif // DAQD_TRUNK_BURT_FILE_HH
diff --git a/src/epics/seq/fixed_size_string.hh b/src/epics/seq/fixed_size_string.hh
index e647760d7..6d88eb325 100644
--- a/src/epics/seq/fixed_size_string.hh
+++ b/src/epics/seq/fixed_size_string.hh
@@ -103,6 +103,19 @@ namespace embedded
return *this;
}
+ fixed_string&
+ operator+=( char in ) noexcept
+ {
+ if ( in && ( remaining() > 0 ) )
+ {
+ size_type cur_size = size( );
+ data_[ cur_size ] = in;
+ data_[ cur_size + 1 ] = 0;
+ }
+ return *this;
+ }
+
+
fixed_string&
operator+=( const char* in ) noexcept
{
diff --git a/src/epics/seq/main.cc b/src/epics/seq/main.cc
index a3dc2d222..96d075ad4 100644
--- a/src/epics/seq/main.cc
+++ b/src/epics/seq/main.cc
@@ -1066,7 +1066,7 @@ bool writeTable2File(const char *burtdir,
if(myTable[ii].datatype == SDF_NUM) {
fprintf(csFile,"%s %d %.*e %s %d\n",entry.chname,1,precision,entry.data.chval,monitorstring.c_str(),entry.initialized);
} else {
- encodeBURTString(entry.data.strval, burtString);
+ BURT::encodeBURTString(entry.data.strval, burtString);
fprintf(csFile,"%s %d %s %s %d\n",entry.chname,1,burtString.c_str(),monitorstring.c_str(),entry.initialized);
}
break;
@@ -1075,7 +1075,7 @@ bool writeTable2File(const char *burtdir,
if(entry.datatype == SDF_NUM) {
fprintf(csFile,"%s %d %.*e %s\n",entry.chname,1,precision,entry.data.chval,monitorstring.c_str());
} else {
- encodeBURTString(entry.data.strval, burtString );
+ BURT::encodeBURTString(entry.data.strval, burtString );
fprintf(csFile,"%s %d %s %s\n",entry.chname,1,burtString.c_str(),monitorstring.c_str());
}
}
@@ -1084,7 +1084,7 @@ bool writeTable2File(const char *burtdir,
if(entry.datatype == SDF_NUM) {
fprintf(csFile,"%s %d %.*e\n",entry.chname,1,precision,entry.data.chval);
} else {
- encodeBURTString(entry.data.strval, burtString );
+ BURT::encodeBURTString(entry.data.strval, burtString );
fprintf(csFile,"%s %d %s \n",entry.chname,1,burtString.c_str());
}
break;
@@ -1935,7 +1935,6 @@ int readConfig( const char *pref, ///< EPICS channel prefix from EPICS environm
char c=0;
int ii=0;
int lock=0;
- char s1[128],s2[128],s3[128],s4[128],s5[128],s6[128];
char ls[6][64];
ADDRESS paddr;
dbAddr reloadDbAddr;
@@ -1962,9 +1961,6 @@ int readConfig( const char *pref, ///< EPICS channel prefix from EPICS environm
const static char* header_start = "--- Start";
const static char* header_end = "--- End";
-
-
- s1[0]=s2[0]=s3[0]=s4[0]=s5[0]=s6[0]='\0';
line[0]='\0';
ifo[0]='\0';
fname[0]=errMsg[0]='\0';
@@ -2001,9 +1997,6 @@ int readConfig( const char *pref, ///< EPICS channel prefix from EPICS environm
}
chNumP = 0;
alarmCnt = 0;
- // Put dummy in s4 as this column may or may not exist.
- strcpy(s4,"x");
- bzero(s3,sizeof(s3));
strncpy(ifo,pref,3);
chNotFound = 0;
while(fgets(line,sizeof line,cdf) != nullptr)
@@ -2026,22 +2019,22 @@ int readConfig( const char *pref, ///< EPICS channel prefix from EPICS environm
isalarm = 0;
lineCnt ++;
- strcpy(s4,"x");
- argcount = parseLine(line,sizeof(s1),s1,s2,s3,s4,s5,s6);
- if (strcmp(s4, "") == 0) strcpy(s4, "0");
- if(argcount == -1) {
- readErrTable[rderror].chname = s1;
- readErrTable[rderror].burtset = "Improper quotations ";
- readErrTable[rderror].liveset.printf("Line # %d", lineCnt);
- readErrTable[rderror].liveval = 0.0;
- readErrTable[rderror].diff = sdfile;
- readErrTable[rderror].timeset = timestring;
- rderror ++;
- printf("Read error --- %s\n",s1);
- continue;
- }
- // Only 3 = no monit flag
- // >=4 count be monit flag or string with quotes
+ BURT::parsed_line words{};
+ argcount = BURT::parseLine(line, words);
+ // Only 3 = no monit flag
+ // >=4 count be monit flag or string with quotes
+ if ( argcount < 3 )
+ {
+ continue;
+ }
+ if ( argcount < 4 )
+ {
+ words.push_back(BURT::parsed_word("0"));
+ }
+ const char* s1 = words[0].c_str();
+ const char* s2 = words[1].c_str();
+ const char* s3 = words[2].c_str();
+ const char* s4 = words[3].c_str();
// If 1st three chars match IFO ie checking this this line is not BURT header or channel marked RO
if(
// Don't allow load of SWSTAT or SWMASK, which are set by this program.
@@ -2656,7 +2649,7 @@ void connectCallback(struct connection_handler_args args) {
}
/// Routine to register a channel
-void registerPV(char *PVname)
+void registerPV(const char *PVname)
{
long status=0;
chid chid1;
@@ -2712,16 +2705,17 @@ int daqToSDFDataType(int daqtype) {
void parseChannelListReq(char *fname) {
FILE *f = 0;
char line[128];
- char s1[128],s2[128],s3[128],s4[128],s5[128],s6[128];
int argcount = 0;
int in_header = 0;
const static char* header_start = "--- Start";
const static char* header_end = "--- End";
- line[0]=s1[0]=s2[0]=s3[0]=s4[0]=s5[0]=s6[0]='\0';
+ line[0]='\0';
f = fopen(fname, "r");
if (!f) return;
+ embedded::fixed_size_vector<embedded::fixed_string<128>, 1> words;
+
while (fgets(line, sizeof(line), f) != NULL) {
if (in_header)
{
@@ -2736,13 +2730,13 @@ void parseChannelListReq(char *fname) {
in_header = 1;
continue;
}
- argcount = parseLine(line, sizeof(s1), s1, s2, s3, s4, s5, s6);
+ argcount = BURT::parseLine(line, words);
if (argcount < 1) continue;
- if (strstr(s1,"_SWMASK") != NULL ||
- strstr(s1,"_SDF_NAME") != NULL ||
- strstr(s1,"_SWREQ") != NULL) continue;
- if (isAlarmChannelRaw(s1)) continue;
- registerPV(s1);
+ if (strstr(words.front().c_str(),"_SWMASK") != NULL ||
+ strstr(words.front().c_str(),"_SDF_NAME") != NULL ||
+ strstr(words.front().c_str(),"_SWREQ") != NULL) continue;
+ if (isAlarmChannelRaw(words.front().c_str())) continue;
+ registerPV(words.front().c_str());
}
fclose(f);
}
diff --git a/src/epics/seq/test/test_burtfile.cc b/src/epics/seq/test/test_burtfile.cc
index 8f49b6a16..be9f24231 100644
--- a/src/epics/seq/test/test_burtfile.cc
+++ b/src/epics/seq/test/test_burtfile.cc
@@ -27,11 +27,11 @@ TEST_CASE( "Test encodeBURTString" )
{
embedded::fixed_string< 18 > src( test_case.input );
embedded::fixed_string< 20 > dest;
- encodeBURTString( src, dest );
+ BURT::encodeBURTString( src, dest );
REQUIRE( strcmp( test_case.expected_output, dest.c_str( ) ) == 0 );
}
}
-
+#include <iostream>
TEST_CASE( "Test parseLine" )
{
struct TestCase
@@ -65,27 +65,15 @@ TEST_CASE( "Test parseLine" )
};
for ( const auto& test_case: test_cases)
{
- std::array<char[16], 6> out{};
- std::vector<char> input(test_case.input, test_case.input + strlen(test_case.input) + 1);
- /* Canary goes at the end of each output to check for overflow */
- for (auto& out_entry:out)
- {
- out_entry[sizeof(out[0])-1] = 0x7f;
- }
- int wc = parseLine(input.data(), sizeof(out[0])-1, &out[0][0], &out[1][0], &out[2][0], &out[3][0], &out[4][0],&out[5][0]);
+ embedded::fixed_size_vector<embedded::fixed_string<15>, 6> out{};
+
+ std::cout << test_case.input << "\n";
+ int wc = BURT::parseLine(test_case.input, out);
REQUIRE(wc == test_case.expected_output.size());
+ REQUIRE(wc == out.size());
for (auto i = 0; i < wc; ++i)
{
- REQUIRE(test_case.expected_output[i] == out[i]);
- }
- std::string empty{};
- for (auto i = wc; i < out.size(); ++i)
- {
- REQUIRE( empty == out[i] );
- }
- for (auto& out_entry:out)
- {
- REQUIRE(out_entry[sizeof(out[0])-1] == 0x7f);
+ REQUIRE(out[i] == test_case.expected_output[i].c_str());
}
}
}
\ No newline at end of file
diff --git a/src/epics/seq/test/test_fixed_size_string.cc b/src/epics/seq/test/test_fixed_size_string.cc
index 8792c5317..08920e23b 100644
--- a/src/epics/seq/test/test_fixed_size_string.cc
+++ b/src/epics/seq/test/test_fixed_size_string.cc
@@ -199,6 +199,20 @@ TEST_CASE( "You can append to a fixed size string" )
embedded::fixed_string< 64 > s;
embedded::fixed_string< 6 > single( "0" );
embedded::fixed_string< 6 > five( "00000" );
+
+ for ( int i = 1; i <= 100; ++i )
+ {
+ s += '0';
+ std::size_t expected_zeros =
+ ( i < s.capacity( ) ? i : s.capacity( ) - 1 );
+ REQUIRE( s.size( ) == expected_zeros );
+ std::size_t j = 0;
+ for ( ; j < expected_zeros && s.c_str( )[ j ] == '0'; ++j )
+ {
+ }
+ REQUIRE( j == expected_zeros );
+ }
+ s.clear();
for ( int i = 1; i <= 100; ++i )
{
s += "0";
--
GitLab