Modify the update_status field in struct file to be an enum.

Makes the code a little clearer/cleaner, and solves a problem on systems
where a char is unsigned by default.

9 files changed, 121 insertions, 102 deletions

diff --git a/ChangeLog b/ChangeLog
index 11442c7..78690ea 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,30 @@
+2013-07-14  Paul Smith  <psmith@gnu.org>
+
+	* filedef.h (update_status): Convert UPDATE_STATUS from a char to
+	an enumeration.  Some systems declare "char" to be "unsigned"
+	which broke the code (which expected to be able to use -1 as a
+	flag).  Using magic values was unpleasant, so rather than just
+	force "signed char" I reworked it to use an enum.
+
+	* dep.h (update_goal_chain): Return an update_status value not int.
+	* remake.c (touch_file): Ditto.
+	(update_goal_chain): Track the update_status enum.
+
+	* file.c (enter_file): Use new enumeration values with update_status.
+	(remove_intermediates): Ditto.
+	(print_file): Ditto.
+	* commands.c (execute_file_commands): Ditto.
+	* job.c (reap_children): Ditto.
+	(start_job_command): Ditto.
+	(start_waiting_job): Ditto.
+	* main.c (main): Ditto.
+	* remake.c (update_file): Ditto.
+	(complain): Ditto.
+	(update_file_1): Ditto.
+	(notice_finished_file): Ditto.
+	(remake_file): Ditto.
+	* vmsjobs.c (vmsHandleChildTerm): Ditto.
+
 2013-07-09  Paul Smith  <psmith@gnu.org>
 
 	* implicit.c (pattern_search): Keep a local copy of the number of
diff --git a/commands.c b/commands.c
index 734c505..e83cc9b 100644
--- a/commands.c
+++ b/commands.c
@@ -459,7 +459,7 @@ execute_file_commands (struct file *file)
     {
       /* If there are no commands, assume everything worked.  */
       set_command_state (file, cs_running);
-      file->update_status = 0;
+      file->update_status = us_success;
       notice_finished_file (file);
       return;
     }
diff --git a/dep.h b/dep.h
index a7c74a1..4abfefe 100644
--- a/dep.h
+++ b/dep.h
@@ -87,4 +87,4 @@ void free_dep_chain (struct dep *d);
 void free_ns_chain (struct nameseq *n);
 struct dep *read_all_makefiles (const char **makefiles);
 void eval_buffer (char *buffer, const gmk_floc *floc);
-int update_goal_chain (struct dep *goals);
+enum update_status update_goal_chain (struct dep *goals);
diff --git a/file.c b/file.c
index 284e19a..25ddde7 100644
--- a/file.c
+++ b/file.c
@@ -184,7 +184,7 @@ enter_file (const char *name)
 
   new = xcalloc (sizeof (struct file));
   new->name = new->hname = name;
-  new->update_status = -1;
+  new->update_status = us_none;
 
   if (HASH_VACANT (f))
     {
@@ -378,7 +378,7 @@ remove_intermediates (int sig)
             && !f->secondary && !f->cmd_target)
           {
             int status;
-            if (f->update_status == -1)
+            if (f->update_status == us_none)
               /* If nothing would have created this file yet,
                  don't print an "rm" command for it.  */
               continue;
@@ -1000,23 +1000,18 @@ print_file (const void *item)
     case cs_finished:
       switch (f->update_status)
         {
-        case -1:
+        case us_none:
           break;
-        case 0:
+        case us_success:
           puts (_("#  Successfully updated."));
           break;
-        case 1:
+        case us_question:
           assert (question_flag);
           puts (_("#  Needs to be updated (-q is set)."));
           break;
-        case 2:
+        case us_failed:
           puts (_("#  Failed to be updated."));
           break;
-        default:
-          puts (_("#  Invalid value in 'update_status' member!"));
-          fflush (stdout);
-          fflush (stderr);
-          abort ();
         }
       break;
     default:
diff --git a/filedef.h b/filedef.h
index 73dc037..f458551 100644
--- a/filedef.h
+++ b/filedef.h
@@ -59,11 +59,16 @@ struct file
     FILE_TIMESTAMP mtime_before_update; /* File's modtime before any updating
                                            has been performed.  */
     int command_flags;          /* Flags OR'd in for cmds; see commands.h.  */
-    char update_status;         /* Status of the last attempt to update,
-                                   or -1 if none has been made.  */
+    enum update_status          /* Status of the last attempt to update.  */
+      {
+        us_success = 0,         /* Successfully updated.  Must be 0!  */
+        us_none,                /* No attempt to update has been made.  */
+        us_question,            /* Needs to be updated (-q is is set).  */
+        us_failed               /* Update failed.  */
+      } update_status ENUM_BITFIELD (2);
     enum cmd_state              /* State of the commands.  */
-      {         /* Note: It is important that cs_not_started be zero.  */
-        cs_not_started,         /* Not yet started.  */
+      {
+        cs_not_started = 0,     /* Not yet started.  Must be 0!  */
         cs_deps_running,        /* Dep commands running.  */
         cs_running,             /* Commands running.  */
         cs_finished             /* Commands finished.  */
diff --git a/job.c b/job.c
index 8b1a8df..68f1f26 100644
--- a/job.c
+++ b/job.c
@@ -1116,7 +1116,7 @@ reap_children (int block, int err)
           if (!dontcare)
             child_error (c, exit_code, exit_sig, coredump, 0);
 
-          c->file->update_status = 2;
+          c->file->update_status = us_failed;
           if (delete_on_error == -1)
             {
               struct file *f = lookup_file (".DELETE_ON_ERROR");
@@ -1143,7 +1143,7 @@ reap_children (int block, int err)
                      Since there are more commands that wanted to be run,
                      the target was not completely remade.  So we treat
                      this as if a command had failed.  */
-                  c->file->update_status = 2;
+                  c->file->update_status = us_failed;
                 }
               else
                 {
@@ -1170,7 +1170,7 @@ reap_children (int block, int err)
                     continue;
                 }
 
-              if (c->file->update_status != 0)
+              if (c->file->update_status != us_success)
                 /* We failed to start the commands.  */
                 delete_child_targets (c);
             }
@@ -1178,7 +1178,7 @@ reap_children (int block, int err)
             /* There are no more commands.  We got through them all
                without an unignored error.  Now the target has been
                successfully updated.  */
-            c->file->update_status = 0;
+            c->file->update_status = us_success;
         }
 
       /* When we get here, all the commands for c->file are finished.  */
@@ -1188,7 +1188,7 @@ reap_children (int block, int err)
       sync_output (c);
 #endif /* OUTPUT_SYNC */
 
-      /* At this point c->file->update_status contains 0 or 2.  But
+      /* At this point c->file->update_status is success or failed.  But
          c->file->command_state is still cs_running if all the commands
          ran; notice_finish_file looks for cs_running to tell it that
          it's interesting to check the file's modtime again now.  */
@@ -1476,7 +1476,7 @@ start_job_command (struct child *child)
       free (argv[0]);
       free (argv);
 #endif
-      child->file->update_status = 1;
+      child->file->update_status = us_question;
       notice_finished_file (child->file);
       return;
     }
@@ -1509,7 +1509,7 @@ start_job_command (struct child *child)
           /* No more commands.  Make sure we're "running"; we might not be if
              (e.g.) all commands were skipped due to -n.  */
           set_command_state (child->file, cs_running);
-          child->file->update_status = 0;
+          child->file->update_status = us_success;
           notice_finished_file (child->file);
         }
       return;
@@ -1908,7 +1908,7 @@ start_job_command (struct child *child)
   return;
 
  error:
-  child->file->update_status = 2;
+  child->file->update_status = us_failed;
   notice_finished_file (child->file);
   return;
 }
@@ -1963,7 +1963,7 @@ start_waiting_job (struct child *c)
 
     case cs_not_started:
       /* All the command lines turned out to be empty.  */
-      f->update_status = 0;
+      f->update_status = us_success;
       /* FALLTHROUGH */
 
     case cs_finished:
diff --git a/main.c b/main.c
index 83a95db..d6ea34d 100644
--- a/main.c
+++ b/main.c
@@ -1719,7 +1719,7 @@ main (int argc, char **argv, char **envp)
             {
               struct file *f = enter_file (strcache_add (stdin_nm));
               f->updated = 1;
-              f->update_status = 0;
+              f->update_status = us_success;
               f->command_state = cs_finished;
               /* Can't be intermediate, or it'll be removed too early for
                  make re-exec.  */
@@ -2086,7 +2086,7 @@ main (int argc, char **argv, char **envp)
           struct file *f = enter_file (*p);
           f->last_mtime = f->mtime_before_update = OLD_MTIME;
           f->updated = 1;
-          f->update_status = 0;
+          f->update_status = us_success;
           f->command_state = cs_finished;
         }
     }
@@ -2113,7 +2113,7 @@ main (int argc, char **argv, char **envp)
       char **nargv;
       int nargc;
       int orig_db_level = db_level;
-      int status;
+      enum update_status status;
 
       if (! ISDB (DB_MAKEFILES))
         db_level = DB_NONE;
@@ -2180,18 +2180,18 @@ main (int argc, char **argv, char **envp)
 
       switch (status)
         {
-        case 1:
+        case us_question:
           /* The only way this can happen is if the user specified -q and asked
            * for one of the makefiles to be remade as a target on the command
            * line.  Since we're not actually updating anything with -q we can
            * treat this as "did nothing".
            */
 
-        case -1:
+        case us_none:
           /* Did nothing.  */
           break;
 
-        case 2:
+        case us_failed:
           /* Failed to update.  Figure out if we care.  */
           {
             /* Nonzero if any makefile was successfully remade.  */
@@ -2211,7 +2211,7 @@ main (int argc, char **argv, char **envp)
                 if (d->file->updated)
                   {
                     /* This makefile was updated.  */
-                    if (d->file->update_status == 0)
+                    if (d->file->update_status == us_success)
                       {
                         /* It was successfully updated.  */
                         any_remade |= (file_mtime_no_search (d->file)
@@ -2260,7 +2260,7 @@ main (int argc, char **argv, char **envp)
             break;
           }
 
-        case 0:
+        case us_success:
         re_exec:
           /* Updated successfully.  Re-exec ourselves.  */
 
@@ -2398,25 +2398,19 @@ main (int argc, char **argv, char **envp)
                child process including all file handles and to wait for its
                termination. */
             int pid;
-            int status;
+            int r;
             pid = child_execute_job (0, 1, nargv, environ);
 
             /* is this loop really necessary? */
             do {
-              pid = wait (&status);
+              pid = wait (&r);
             } while (pid <= 0);
             /* use the exit code of the child process */
-            exit (WIFEXITED(status) ? WEXITSTATUS(status) : EXIT_FAILURE);
+            exit (WIFEXITED(r) ? WEXITSTATUS(r) : EXIT_FAILURE);
           }
 #else
           exec_command (nargv, environ);
 #endif
-          /* NOTREACHED */
-
-        default:
-#define BOGUS_UPDATE_STATUS 0
-          assert (BOGUS_UPDATE_STATUS);
-          break;
         }
 
       db_level = orig_db_level;
@@ -2511,27 +2505,23 @@ main (int argc, char **argv, char **envp)
   DB (DB_BASIC, (_("Updating goal targets....\n")));
 
   {
-    int status;
-
     switch (update_goal_chain (goals))
     {
-      case -1:
+      case us_none:
         /* Nothing happened.  */
-      case 0:
-        /* Updated successfully.  */
-        status = makefile_status;
+        /* FALLTHROUGH */
+      case us_success:
+        /* Keep the previous result.  */
         break;
-      case 1:
+      case us_question:
         /* We are under -q and would run some commands.  */
-        status = MAKE_TROUBLE;
+        makefile_status = MAKE_TROUBLE;
         break;
-      case 2:
+      case us_failed:
         /* Updating failed.  POSIX.2 specifies exit status >1 for this;
            but in VMS, there is only success and failure.  */
-        status = MAKE_FAILURE;
+        makefile_status = MAKE_FAILURE;
         break;
-      default:
-        abort ();
     }
 
     /* If we detected some clock skew, generate one last warning */
@@ -2540,7 +2530,7 @@ main (int argc, char **argv, char **envp)
              _("warning:  Clock skew detected.  Your build may be incomplete."));
 
     /* Exit.  */
-    die (status);
+    die (makefile_status);
   }
 
   /* NOTREACHED */
diff --git a/remake.c b/remake.c
index 06e47bc..713acfd 100644
--- a/remake.c
+++ b/remake.c
@@ -62,7 +62,7 @@ static int update_file (struct file *file, unsigned int depth);
 static int update_file_1 (struct file *file, unsigned int depth);
 static int check_dep (struct file *file, unsigned int depth,
                       FILE_TIMESTAMP this_mtime, int *must_make_ptr);
-static int touch_file (struct file *file);
+static enum update_status touch_file (struct file *file);
 static void remake_file (struct file *file);
 static FILE_TIMESTAMP name_mtime (const char *name);
 static const char *library_search (const char *lib, FILE_TIMESTAMP *mtime_ptr);
@@ -76,11 +76,11 @@ static const char *library_search (const char *lib, FILE_TIMESTAMP *mtime_ptr);
    targets, and we should only make one goal at a time and return as soon as
    one goal whose 'changed' member is nonzero is successfully made.  */
 
-int
+enum update_status
 update_goal_chain (struct dep *goals)
 {
   int t = touch_flag, q = question_flag, n = just_print_flag;
-  int status = -1;
+  enum update_status status = us_none;
 
 #define MTIME(file) (rebuilding_makefiles ? file_mtime_no_search (file) \
                      : file_mtime (file))
@@ -130,7 +130,7 @@ update_goal_chain (struct dep *goals)
                file = file->prev)
             {
               unsigned int ocommands_started;
-              int x;
+              int fail;
 
               file->dontcare = g->dontcare;
 
@@ -152,7 +152,7 @@ update_goal_chain (struct dep *goals)
                  actually run.  */
               ocommands_started = commands_started;
 
-              x = update_file (file, rebuilding_makefiles ? 1 : 0);
+              fail = update_file (file, rebuilding_makefiles ? 1 : 0);
               check_renamed (file);
 
               /* Set the goal's 'changed' flag if any commands were started
@@ -166,9 +166,9 @@ update_goal_chain (struct dep *goals)
                  STATUS as it is if no updating was done.  */
 
               stop = 0;
-              if ((x != 0 || file->updated) && status < 1)
+              if ((fail || file->updated) && status < us_question)
                 {
-                  if (file->update_status != 0)
+                  if (file->update_status != us_success)
                     {
                       /* Updating failed, or -q triggered.  The STATUS value
                          tells our caller which.  */
@@ -195,7 +195,7 @@ update_goal_chain (struct dep *goals)
                              enter an infinite loop.  */
                           if (!rebuilding_makefiles
                               || (!just_print_flag && !question_flag))
-                            status = 0;
+                            status = us_success;
                           if (rebuilding_makefiles && file->dontcare)
                             /* This is a default makefile; stop remaking.  */
                             stop = 1;
@@ -222,10 +222,10 @@ update_goal_chain (struct dep *goals)
                  print a message saying nothing needs doing.  */
 
               if (!rebuilding_makefiles
-                  /* If the update_status is zero, we updated successfully
+                  /* If the update_status is success, we updated successfully
                      or not at all.  G->changed will have been set above if
                      any commands were actually started for this goal.  */
-                  && file->update_status == 0 && !g->changed
+                  && file->update_status == us_success && !g->changed
                   /* Never give a message under -s or -q.  */
                   && !silent_flag && !question_flag)
                 message (1, ((file->phony || file->cmds == 0)
@@ -271,7 +271,7 @@ update_goal_chain (struct dep *goals)
 }
 
 /* If FILE is not up to date, execute the commands for it.
-   Return 0 if successful, 1 if unsuccessful;
+   Return 0 if successful, non-0 if unsuccessful;
    but with some flag settings, just call 'exit' if unsuccessful.
 
    DEPTH is the depth in recursions of this function.
@@ -285,7 +285,7 @@ update_goal_chain (struct dep *goals)
 static int
 update_file (struct file *file, unsigned int depth)
 {
-  register int status = 0;
+  int status = 0;
   register struct file *f;
 
   f = file->double_colon ? file->double_colon : file;
@@ -297,9 +297,10 @@ update_file (struct file *file, unsigned int depth)
   if (f->considered == considered)
     {
       /* Check for the case where a target has been tried and failed but
-         the diagnostics hasn't been issued. If we need the diagnostics
+         the diagnostics haven't been issued. If we need the diagnostics
          then we will have to continue. */
-      if (!(f->updated && f->update_status > 0 && !f->dontcare && f->no_diag))
+      if (!(f->updated && f->update_status > us_none
+            && !f->dontcare && f->no_diag))
         {
           DBF (DB_VERBOSE, _("Pruning file '%s'.\n"));
           return f->command_state == cs_finished ? f->update_status : 0;
@@ -363,7 +364,7 @@ complain (struct file *file)
 
   for (d = file->deps; d != 0; d = d->next)
     {
-      if (d->file->updated && d->file->update_status > 0 && file->no_diag)
+      if (d->file->updated && d->file->update_status > us_none && file->no_diag)
         {
           complain (d->file);
           break;
@@ -395,7 +396,8 @@ complain (struct file *file)
     }
 }
 
-/* Consider a single 'struct file' and update it as appropriate.  */
+/* Consider a single 'struct file' and update it as appropriate.
+   Return 0 on success, or non-0 on failure.  */
 
 static int
 update_file_1 (struct file *file, unsigned int depth)
@@ -412,7 +414,7 @@ update_file_1 (struct file *file, unsigned int depth)
 
   if (file->updated)
     {
-      if (file->update_status > 0)
+      if (file->update_status > us_none)
         {
           DBF (DB_VERBOSE,
                _("Recently tried and failed to update file '%s'.\n"));
@@ -577,7 +579,7 @@ update_file_1 (struct file *file, unsigned int depth)
             while (f != 0);
           }
 
-          if (dep_status != 0 && !keep_going_flag)
+          if (dep_status && !keep_going_flag)
             break;
 
           if (!running)
@@ -638,7 +640,7 @@ update_file_1 (struct file *file, unsigned int depth)
               while (f != 0);
             }
 
-            if (dep_status != 0 && !keep_going_flag)
+            if (dep_status && !keep_going_flag)
               break;
 
             if (!running)
@@ -664,7 +666,7 @@ update_file_1 (struct file *file, unsigned int depth)
 
   if (dep_status != 0)
     {
-      file->update_status = dep_status;
+      file->update_status = us_failed;
       notice_finished_file (file);
 
       --depth;
@@ -820,17 +822,16 @@ update_file_1 (struct file *file, unsigned int depth)
 
   switch (file->update_status)
     {
-    case 2:
+    case us_failed:
       DBF (DB_BASIC, _("Failed to remake target file '%s'.\n"));
       break;
-    case 0:
+    case us_success:
       DBF (DB_BASIC, _("Successfully remade target file '%s'.\n"));
       break;
-    case 1:
+    case us_question:
       DBF (DB_BASIC, _("Target file '%s' needs to be remade under -q.\n"));
       break;
-    default:
-      assert (file->update_status >= 0 && file->update_status <= 2);
+    case us_none:
       break;
     }
 
@@ -842,7 +843,7 @@ update_file_1 (struct file *file, unsigned int depth)
    files listed in its 'also_make' member.  Under -t, this function also
    touches FILE.
 
-   On return, FILE->update_status will no longer be -1 if it was.  */
+   On return, FILE->update_status will no longer be us_none if it was.  */
 
 void
 notice_finished_file (struct file *file)
@@ -856,12 +857,12 @@ notice_finished_file (struct file *file)
 
   if (touch_flag
       /* The update status will be:
-                -1      if this target was not remade;
-                0       if 0 or more commands (+ or ${MAKE}) were run and won;
-                1       if some commands were run and lost.
+           us_success   if 0 or more commands (+ or ${MAKE}) were run and won;
+           us_none      if this target was not remade;
+           >us_none     if some commands were run and lost.
          We touch the target if it has commands which either were not run
          or won when they ran (i.e. status is 0).  */
-      && file->update_status == 0)
+      && file->update_status == us_success)
     {
       if (file->cmds != 0 && file->cmds->any_recurse)
         {
@@ -876,7 +877,7 @@ notice_finished_file (struct file *file)
         {
         have_nonrecursing:
           if (file->phony)
-            file->update_status = 0;
+            file->update_status = us_success;
           /* According to POSIX, -t doesn't affect targets with no cmds.  */
           else if (file->cmds != 0)
             {
@@ -950,7 +951,7 @@ notice_finished_file (struct file *file)
           f->last_mtime = max_mtime;
     }
 
-  if (ran && file->update_status != -1)
+  if (ran && file->update_status != us_none)
     /* We actually tried to update FILE, which has
        updated its also_make's as well (if it worked).
        If it didn't work, it wouldn't work again for them.
@@ -968,10 +969,10 @@ notice_finished_file (struct file *file)
              never be done because the target is already updated.  */
           f_mtime (d->file, 0);
       }
-  else if (file->update_status == -1)
+  else if (file->update_status == us_none)
     /* Nothing was done for FILE, but it needed nothing done.
        So mark it now as "succeeded".  */
-    file->update_status = 0;
+    file->update_status = us_success;
 }
 
 /* Check whether another file (whose mtime is THIS_MTIME) needs updating on
@@ -1112,11 +1113,12 @@ check_dep (struct file *file, unsigned int depth,
   return dep_status;
 }
 
-/* Touch FILE.  Return zero if successful, one if not.  */
+/* Touch FILE.  Return us_success if successful, us_failed if not.  */
 
-#define TOUCH_ERROR(call) return (perror_with_name (call, file->name), 1)
+#define TOUCH_ERROR(call) do{ perror_with_name ((call), file->name);    \
+                              return us_failed; }while(0)
 
-static int
+static enum update_status
 touch_file (struct file *file)
 {
   if (!silent_flag)
@@ -1124,11 +1126,11 @@ touch_file (struct file *file)
 
   /* Print-only (-n) takes precedence over touch (-t).  */
   if (just_print_flag)
-    return 0;
+    return us_success;
 
 #ifndef NO_ARCHIVES
   if (ar_name (file->name))
-    return ar_touch (file->name);
+    return ar_touch (file->name) ? us_failed : us_success;
   else
 #endif
     {
@@ -1165,7 +1167,7 @@ touch_file (struct file *file)
         }
     }
 
-  return 0;
+  return us_success;
 }
 
 /* Having checked and updated the dependencies of FILE,
@@ -1179,17 +1181,17 @@ remake_file (struct file *file)
     {
       if (file->phony)
         /* Phony target.  Pretend it succeeded.  */
-        file->update_status = 0;
+        file->update_status = us_success;
       else if (file->is_target)
         /* This is a nonexistent target file we cannot make.
            Pretend it was successfully remade.  */
-        file->update_status = 0;
+        file->update_status = us_success;
       else
         {
           /* This is a dependency file we cannot remake.  Fail.  */
           if (!rebuilding_makefiles || !file->dontcare)
             complain (file);
-          file->update_status = 2;
+          file->update_status = us_failed;
         }
     }
   else
@@ -1204,7 +1206,7 @@ remake_file (struct file *file)
         }
 
       /* This tells notice_finished_file it is ok to touch the file.  */
-      file->update_status = 0;
+      file->update_status = us_success;
     }
 
   /* This does the touching under -t.  */
diff --git a/vmsjobs.c b/vmsjobs.c
index 7e685c8..8d30157 100644
--- a/vmsjobs.c
+++ b/vmsjobs.c
@@ -150,7 +150,7 @@ vmsHandleChildTerm(struct child *child)
 	/* The commands failed.  Write an error message,
 	   delete non-precious targets, and abort.  */
 	child_error (c, c->cstatus, 0, 0, 0);
-	c->file->update_status = 1;
+	c->file->update_status = us_failed;
 	delete_child_targets (c);
       }
     else
@@ -173,7 +173,7 @@ vmsHandleChildTerm(struct child *child)
 	    break;
 
 	  case cs_finished:
-	    if (c->file->update_status != 0) {
+	    if (c->file->update_status != us_success) {
 		/* We failed to start the commands.  */
 		delete_child_targets (c);
 	    }