From 4b81f5ca920d716c08430583f5edb2c125f1f123 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Sun, 14 Jul 2013 19:18:21 -0400 Subject: 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. --- ChangeLog | 27 +++++++++++++++++++ commands.c | 2 +- dep.h | 2 +- file.c | 17 +++++------- filedef.h | 13 ++++++--- job.c | 18 ++++++------- main.c | 50 ++++++++++++++-------------------- remake.c | 90 ++++++++++++++++++++++++++++++++------------------------------ vmsjobs.c | 4 +-- 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 + + * 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 * 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); } -- cgit v1.2.3