#1518: Replace internal expression parser with Piraha ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: sbrandt Type: defect | Status: review Priority: minor | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: postrelease ---------------------+------------------------------------------------------
Comment (by rhaas):
I can only review code style and some general stuff since I do not actually understand what the Piraha parser does.
Comments:
* line 2312 ff: {{{ 2312 int n=0; 2313 while(temp[n] != 0) 2314 n++; }}} is just a strlen. So strlen should be used. Since the intend seems to be to try and avoid calling strlen() after each iteration, the most convenient way may be: {{{ for (unsigned int p = 0, n = strlen(temp); p < n; p++) }}} the way eg Carpet caches the end() iterator for C++ style iterations. * there's a number of commented out lines that should not be committed (eg line 2845 of src/main/Groups.c, line 2223 of src/main/Parameters.c) * src/piraha/Makefile seems to hard-code compiler options for -O2 and -g. This should not be. * the patch redefine Util_StrCmpi. I don't understand why this is done. If actually required, I think it should be done in a separate commit. We may not even need Util_StrCmpi anymore since POSIX.1-2001 and we could officially add POSIX.1-2001 to Cactus' requirements.