From e25be527a65a4a40d4ad720911d7aac9f817dee4 Mon Sep 17 00:00:00 2001
From: jay <jay>
Date: Fri, 23 Dec 2005 16:55:09 +0000
Subject: [PATCH] Savannah bug #15271: more helpful error messages for cases
 where there is a missing expression

---
 find/find.c    | 17 ++++++++++++++---
 find/ftsfind.c | 19 ++++++++++++++++---
 find/tree.c    | 25 ++++++++++++++++++++++---
 3 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/find/find.c b/find/find.c
index e1e46bf..21b42ab 100644
--- a/find/find.c
+++ b/find/find.c
@@ -261,6 +261,7 @@ main (int argc, char **argv)
   
   /* Enclose the expression in `( ... )' so a default -print will
      apply to the whole expression. */
+/* XXX: beginning of bit we need factor out of both find.c and ftsfind.c */
   entry_open  = find_parser("(");
   entry_close = find_parser(")");
   entry_print = find_parser("print");
@@ -360,10 +361,19 @@ main (int argc, char **argv)
    */
   if (cur_pred != NULL)
     {
-      if (0 == strcmp(cur_pred->p_name, ")"))
-	error (1, 0, _("you have too many '%s'"), cur_pred->p_name);
+      /* cur_pred->p_name is often NULL here */
+      if (cur_pred->pred_func == pred_close)
+	{
+	  /* e.g. "find \( -true \) \)" */
+	  error (1, 0, _("you have too many ')'"), cur_pred->p_name);
+	}
       else
-	error (1, 0, _("unexpected extra predicate '%s'"), cur_pred->p_name);
+	{
+	  if (cur_pred->p_name)
+	    error (1, 0, _("unexpected extra predicate '%s'"), cur_pred->p_name);
+	  else
+	    error (1, 0, _("unexpected extra predicate"));
+	}
     }
   
 #ifdef	DEBUG
@@ -386,6 +396,7 @@ main (int argc, char **argv)
   print_optlist(stderr, eval_tree);
   fprintf(stderr, "\n");
 #endif /* DEBUG */
+/* XXX: end of bit we need factor out of both find.c and ftsfind.c */
 
   /* safely_chdir() needs to check that it has ended up in the right place. 
    * To avoid bailing out when something gets automounted, it checks if 
diff --git a/find/ftsfind.c b/find/ftsfind.c
index 6567634..4255dad 100644
--- a/find/ftsfind.c
+++ b/find/ftsfind.c
@@ -542,6 +542,7 @@ main (int argc, char **argv)
       /* Do nothing. */ ;
     }
   
+/* XXX: beginning of bit we need factor out of both find.c and ftsfind.c */
   /* Enclose the expression in `( ... )' so a default -print will
      apply to the whole expression. */
   entry_open  = find_parser("(");
@@ -640,10 +641,19 @@ main (int argc, char **argv)
    */
   if (cur_pred != NULL)
     {
-      if (0 == strcmp(cur_pred->p_name, ")"))
-	error (1, 0, _("you have too many '%s'"), cur_pred->p_name);
+      /* cur_pred->p_name is often NULL here */
+      if (cur_pred->pred_func == pred_close)
+	{
+	  /* e.g. "find \( -true \) \)" */
+	  error (1, 0, _("you have too many ')'"), cur_pred->p_name);
+	}
       else
-	error (1, 0, _("unexpected extra predicate '%s'"), cur_pred->p_name);
+	{
+	  if (cur_pred->p_name)
+	    error (1, 0, _("unexpected extra predicate '%s'"), cur_pred->p_name);
+	  else
+	    error (1, 0, _("unexpected extra predicate"));
+	}
     }
   
 #ifdef	DEBUG
@@ -667,6 +677,9 @@ main (int argc, char **argv)
   fprintf(stderr, "\n");
 #endif /* DEBUG */
 
+
+  /* XXX: end of bit we need factor out of both find.c and ftsfind.c */
+
   /* safely_chdir() needs to check that it has ended up in the right place. 
    * To avoid bailing out when something gets automounted, it checks if 
    * the target directory appears to have had a directory mounted on it as
diff --git a/find/tree.c b/find/tree.c
index 0f66169..e24d761 100644
--- a/find/tree.c
+++ b/find/tree.c
@@ -64,6 +64,7 @@ get_expr (struct predicate **input,
 	  const struct predicate* prev_pred)
 {
   struct predicate *next = NULL;
+  struct predicate *this_pred = (*input);
 
   if (*input == NULL)
     error (1, 0, _("invalid expression"));
@@ -75,15 +76,25 @@ get_expr (struct predicate **input,
       break;
 
     case BI_OP:
-      error (1, 0, _("invalid expression; you have used a binary operator with nothing before it."));
+      /* e.g. "find . -a" */
+      error (1, 0, _("invalid expression; you have used a binary operator '%s' with nothing before it."), this_pred->p_name);
       break;
 
     case CLOSE_PAREN:
-      if ( (*input)->artificial )
+      if ((UNI_OP == prev_pred->p_type
+	  || BI_OP == prev_pred->p_type)
+	  && !this_pred->artificial)
+	{
+	  /* e.g. "find \( -not \)" or "find \( -true -a \" */
+	  error(1, 0, _("expected an expression between '%s' and ')'"),
+		prev_pred->p_name);
+	}
+      else if ( (*input)->artificial )
 	{
 	  /* We have reached the end of the user-supplied predicates
 	   * unexpectedly. 
 	   */
+	  /* e.g. "find . -true -a" */
 	  error (1, 0, _("expected an expression after '%s'"), prev_pred->p_name);
 	}
       else
@@ -104,6 +115,14 @@ get_expr (struct predicate **input,
       break;
 
     case OPEN_PAREN:
+      if ( (NULL == (*input)->pred_next) || (*input)->pred_next->artificial )
+	{
+	  /* user typed something like "find . (", and so the ) we are
+	   * looking at is from the artificial "( ) -print" that we
+	   * add.
+	   */
+	  error (1, 0, _("invalid expression; expected to find a ')' but didn't see one.  Perhaps you need an extra predicate after '%s'"), this_pred->p_name);
+	}
       prev_pred = (*input);
       *input = (*input)->pred_next;
       if ( (*input)->p_type == CLOSE_PAREN )
@@ -116,7 +135,7 @@ get_expr (struct predicate **input,
 	error (1, 0, _("invalid expression; I was expecting to find a ')' somewhere but did not see one."));
       *input = (*input)->pred_next;	/* move over close */
       break;
-
+      
     default:
       error (1, 0, _("oops -- invalid expression type!"));
       break;
-- 
2.11.4.GIT