Remove some hardcoded selection string buffers

This removes some arbitrary limitations on the length of the input
string.  The error handling would still be in the need of a bigger
overhaul (all of these were in the error handling parts).

Fixes #1538, related to #655.

Change-Id: I1112b5ccae526cf3b8fd079bbabf8d7f61e52635

diff --git a/src/gromacs/selection/params.cpp b/src/gromacs/selection/params.cpp
index 28686b059e50a5495ae39271fd832b176240516c..e0305da77297c723957ef572e65a65e5ae269e96 100644 (file)
--- a/src/gromacs/selection/params.cpp
+++ b/src/gromacs/selection/params.cpp
@@ -616,7 +616,7 @@ add_child(const SelectionTreeElementPointer &root, gmx_ana_selparam_t *param,
         // FIXME: Use exceptions.
         return SelectionTreeElementPointer();
     }
-    _gmx_selelem_update_flags(child, scanner);
+    _gmx_selelem_update_flags(child);
     if ((child->flags & SEL_DYNAMIC) && !(param->flags & SPAR_DYNAMIC))
     {
         _gmx_selparser_error(scanner, "dynamic values not supported");

diff --git a/src/gromacs/selection/parsetree.cpp b/src/gromacs/selection/parsetree.cpp
index a40798df3505c731158290a505e76d9500c5ee64..3035805bd4e11ce0f19f0631faff4351c6952f91 100644 (file)
--- a/src/gromacs/selection/parsetree.cpp
+++ b/src/gromacs/selection/parsetree.cpp
@@ -261,7 +261,7 @@ _gmx_selparser_error(yyscan_t scanner, const char *fmt, ...)
     char    buf[1024];
     va_list ap;
     va_start(ap, fmt);
-    vsprintf(buf, fmt, ap);
+    vsnprintf(buf, 1024, fmt, ap);
     va_end(ap);
     errors->append(buf);
 }
@@ -322,8 +322,6 @@ SelectionParserParameter::SelectionParserParameter(
 
 /*!
  * \param[in,out] sel  Root of the selection element tree to initialize.
- * \param[in]     scanner Scanner data structure.
- * \returns       0 on success, an error code on error.
  *
  * Propagates the \ref SEL_DYNAMIC flag from the children of \p sel to \p sel
  * (if any child of \p sel is dynamic, \p sel is also marked as such).
@@ -340,8 +338,7 @@ SelectionParserParameter::SelectionParserParameter(
  * operation does not descend beyond such elements.
  */
 void
-_gmx_selelem_update_flags(const gmx::SelectionTreeElementPointer &sel,
-                          yyscan_t                                scanner)
+_gmx_selelem_update_flags(const gmx::SelectionTreeElementPointer &sel)
 {
     bool                bUseChildType = false;
     bool                bOnlySingleChildren;
@@ -409,7 +406,7 @@ _gmx_selelem_update_flags(const gmx::SelectionTreeElementPointer &sel,
     while (child)
     {
         /* Update the child */
-        _gmx_selelem_update_flags(child, scanner);
+        _gmx_selelem_update_flags(child);
         /* Propagate the dynamic and unsorted flags */
         sel->flags |= (child->flags & (SEL_DYNAMIC | SEL_UNSORTED));
         /* Propagate the type flag if necessary and check for problems */
@@ -418,9 +415,9 @@ _gmx_selelem_update_flags(const gmx::SelectionTreeElementPointer &sel,
             if ((sel->flags & SEL_VALTYPEMASK)
                 && !(sel->flags & child->flags & SEL_VALTYPEMASK))
             {
-                _gmx_selparser_error(scanner, "invalid combination of selection expressions");
-                // FIXME: Use an exception.
-                return;
+                // TODO: Recollect when this is triggered, and whether the type
+                // is appropriate.
+                GMX_THROW(gmx::InvalidInputError("Invalid combination of selection expressions"));
             }
             sel->flags |= (child->flags & SEL_VALTYPEMASK);
         }
@@ -990,11 +987,6 @@ _gmx_sel_init_selection(const char                             *name,
                         const gmx::SelectionTreeElementPointer &sel,
                         yyscan_t                                scanner)
 {
-    gmx::MessageStringCollector *errors = _gmx_sel_lexer_error_reporter(scanner);
-    char  buf[1024];
-    sprintf(buf, "In selection '%s'", _gmx_sel_lexer_pselstr(scanner));
-    gmx::MessageStringContext  context(errors, buf);
-
     if (sel->v.type != POS_VALUE)
     {
         /* FIXME: Better handling of this error */
@@ -1009,7 +1001,7 @@ _gmx_sel_init_selection(const char                             *name,
         root->setName(name);
     }
     /* Update the flags */
-    _gmx_selelem_update_flags(root, scanner);
+    _gmx_selelem_update_flags(root);
 
     root->fillNameIfMissing(_gmx_sel_lexer_pselstr(scanner));
 
@@ -1043,12 +1035,7 @@ _gmx_sel_assign_variable(const char                             *name,
     const char                  *pselstr = _gmx_sel_lexer_pselstr(scanner);
     SelectionTreeElementPointer  root;
 
-    gmx::MessageStringCollector *errors = _gmx_sel_lexer_error_reporter(scanner);
-    char  buf[1024];
-    sprintf(buf, "In selection '%s'", pselstr);
-    gmx::MessageStringContext  context(errors, buf);
-
-    _gmx_selelem_update_flags(expr, scanner);
+    _gmx_selelem_update_flags(expr);
     /* Check if this is a constant non-group value */
     if (expr->type == SEL_CONST && expr->v.type != GROUP_VALUE)
     {
@@ -1072,7 +1059,7 @@ _gmx_sel_assign_variable(const char                             *name,
     _gmx_selelem_set_vtype(root->child, expr->v.type);
     root->child->child  = expr;
     /* Update flags */
-    _gmx_selelem_update_flags(root, scanner);
+    _gmx_selelem_update_flags(root);
     /* Add the variable to the symbol table */
     sc->symtab->addVariable(name, root->child);
 finish:

diff --git a/src/gromacs/selection/parsetree.h b/src/gromacs/selection/parsetree.h
index 142fe7e1a6f4e23a3caebb03419ead3bae4cf995..d594f3240d61bb0ac4966fe9a867e953c76e6bb5 100644 (file)
--- a/src/gromacs/selection/parsetree.h
+++ b/src/gromacs/selection/parsetree.h
@@ -383,8 +383,7 @@ _gmx_selparser_handle_exception(void *scanner, const std::exception &ex);
 
 /** Propagates the flags for selection elements. */
 void
-_gmx_selelem_update_flags(const gmx::SelectionTreeElementPointer &sel,
-                          void                                   *scanner);
+_gmx_selelem_update_flags(const gmx::SelectionTreeElementPointer &sel);
 
 /** Initializes the method parameter data of \ref SEL_EXPRESSION and
  * \ref SEL_MODIFIER elements. */