Index: ChangeLog =================================================================== RCS file: /cvsroot/gnats/gnats/gnats/ChangeLog,v retrieving revision 1.308 diff -u -p -r1.308 ChangeLog --- ChangeLog 30 Nov 2004 15:44:37 -0000 1.308 +++ ChangeLog 21 Feb 2005 12:19:38 -0000 @@ -1,3 +1,71 @@ +2005-02-19 Mel Hatzis + + * adm.c (copy_adm_entry): Handle blank metadata fields by correctly + assigning the relevant field in the adm struct to NULL - and thereby + avoid an attempt to xstrdup a NULL value. + + * file-pr.c (createNewPRFile): Avoid a NULL pointer dereference when + there's a blank response time field in the submitters file. + + * gnatsd.c (findUserAccessLevel): Avoid a strlen of NULL when the + gnatsd.user_access file contains a blank 'database-alias' field. + + * gnats.h (rewrite_pr): Made it a static function in edit.c - it + wasn't being used outside this scope anyway. + + * mail.c (get_responsible_address): Avoid a NULL pointer dereference + if the responsible file doesn't have an email address for the + gnats-admin user. + + * index.c, index.h (getPrevPR, setPrevPR, setNextPR): New functions. + The index chain is now doubly linked allowing edited PRs to be + replaced without incurring the overhead of a search through the + chain. As well as offering significantly better performance, this + supports a change to the PR edit functionality whereby we now start + with a copy of the old PR as well as the updated PR instead of + retrieving the old PR (from the index) in the guts of rewrite_pr. + The motivation behind this change is to isolate the index + functionality and move toward a generic backend datastore which + doesn't require the index. + (replaceCurrentPRInIndex): Modified signature. Pass in the PR to be + replaced (as well as the new PR) since we can now use the doubly + linked index chain to replace the PR without searching for it. + + * pr.h (lookup_pr_path): Removed. Replaced by a call to the new + get_pr_from_index () function followed by a call to (a modified) + get_pr_path (). This change allows a single index search to be done + once (early on in a given call stack) to retrieve a PR and + thereafter access the PR file without requiring more index + searches. + (pr_delete): New function. Contains the flat-file specific logic + related to PR deletions which will be moved out into a flat-file + backend when the datastore backend is generalized. + + * pr.c (get_pr_from_index): New function. Retrieve a PR from the + index. + (pr_file_readable): New function. Check if a file can be opened for + reading. + (pr_delete): New function. Flat-file specific logic for delete. + (prExists): Now utilize get_pr_from_index () and get_pr_path () + instead of the deprecated lookup_pr_path (). + (readPRWithNum): Ditto. Also set the PR's next/prev index pointers. + (get_pr_path): New signature. Take in a PR * instead of searching + the index. + (lookup_pr_path): Deprecated. + + * edit.c (applyChangeActions, processFieldChange, sendAuditMail): + Repositioned within the file so as not to require prototypes. + (processPRChanges): Handle NULL values appropriately when undoing + changes to modified read-only fields. + (rewrite_pr, replace_pr, edit_field): rewrite_pr now takes in a copy + of the old pr instead of retrieving it from the index. This is in + preparation for allowing it to be used with more than just the flat + file backend datastore. Also moved some error checking out of + rewrite_pr and simplified it...no need to validate the whole PR + when editing a single field. + (deletePR): Moved the file-related logic into a new pr_delete function + inside pr.c thereby preparing for a general backend datastore. + 2004-11-30 Chad Walstrom * gnatsd.c, gnats-pwconv.c (HAVE_CRYPT_H): Changed HAVE_CRYPT to Index: adm.c =================================================================== RCS file: /cvsroot/gnats/gnats/gnats/adm.c,v retrieving revision 1.20 diff -u -p -r1.20 adm.c --- adm.c 27 Oct 2002 06:21:17 -0000 1.20 +++ adm.c 21 Feb 2005 12:19:38 -0000 @@ -47,7 +47,9 @@ copy_adm_entry (AdmEntry *ent) res->field = ent->field; for (x = 0; x < ent->fieldcount; x++) { - res->admFields[x] = xstrdup (ent->admFields[x]); + res->admFields[x] = (ent->admFields[x] == NULL) + ? NULL + : xstrdup (ent->admFields[x]); } return res; } Index: edit.c =================================================================== RCS file: /cvsroot/gnats/gnats/gnats/edit.c,v retrieving revision 1.60 diff -u -p -r1.60 edit.c --- edit.c 25 Nov 2002 13:58:33 -0000 1.60 +++ edit.c 21 Feb 2005 12:19:39 -0000 @@ -24,38 +24,113 @@ Software Foundation, 59 Temple Place - S #include "query.h" #include "mail.h" -static int applyChangeActions (PR *pr, PR *oldPR, FieldIndex field, - ChangeActions actions, ErrorDesc *err, - FormatNamedParameter *params); - -static int processFieldChange (PR *pr, PR *oldPR, FieldIndex field, - ErrorDesc *err, - const char *editUserEmailAddr, - const char *oldValue, const char *newValue); - /* Euuugh. There's gotta be a better way to keep this around. */ static char *newAuditTrailEntries = NULL; -static void sendAuditMail (PR *pr, PR *oldPR, const char *editUserEmailAddr, - const char *newAuditTrailEntries, - ErrorDesc *err); +static void +sendAuditMail (PR *pr, PR *oldPR, const char *editUserEmailAddr, + const char *newAuditTrailEntries, ErrorDesc *err) +{ + FormatNamedParameter *parms = NULL; + const DatabaseInfo database = pr->database; -int -replace_pr (const DatabaseInfo database, FILE *fp, - const char *editUserEmailAddr, ErrorDesc *err) + parms = allocateNamedParameter ("$EditUserEmailAddr", editUserEmailAddr, + parms); + parms = allocateNamedParameter ("$NewAuditTrail", newAuditTrailEntries, + parms); + + if (strcmp (field_value (pr, RESPONSIBLE (database)), + field_value (oldPR, RESPONSIBLE (database))) != 0) + { + parms + = allocateNamedParameter ("$OldResponsible", + field_value (oldPR, RESPONSIBLE (database)), + parms); + } + + composeMailMessage (pr, oldPR, "audit-mail", parms, NULL, err); + freeFormatParameterList (parms); +} + +/* Add an entry to the AUDIT_TRAIL field. PARAMS are the format + parameters used when composing the entry. FMT is the format of the + audit-trail entry to add. */ + +static int +addAuditTrailEnt (PR *pr, QueryFormat *fmt, + FormatNamedParameter *params, ErrorDesc *err) { - PR *pr = allocPR (database); + char *newAuditString = NULL; + char *finalAuditString; + const char *t; + char *currDate = get_curr_date (); - /* Read the message header in. */ - if (read_header (pr, fp) < 0) + if (fmt == NULL) { - setError (err, CODE_EOF_PR, "Couldn't read PR header"); - return 0; + fmt = getAuditTrailFormat (pr->database); } - read_pr (pr, fp, 0); + /* Format the new audit entry. */ + process_format (NULL, &newAuditString, pr, NULL, fmt, "\n", params); - return rewrite_pr (pr, editUserEmailAddr, err); + /* Squirrel it away, because we'll need to mail it later. */ + append_string (&newAuditTrailEntries, newAuditString); + + /* Now append the formatted string to the audit-trail field. */ + t = field_value (pr, AUDIT_TRAIL (pr->database)); + if (t == NULL) + { + t = ""; + } + finalAuditString = xstrdup (t); + append_string (&finalAuditString, newAuditString); + set_field (pr, AUDIT_TRAIL (pr->database), finalAuditString, err); + + free (finalAuditString); + free (newAuditString); + free (currDate); + + return 0; +} + +static int +applyChangeAction (ChangeActions action, PR *pr, PR *oldPR, FieldIndex field, + ErrorDesc *err, FormatNamedParameter *params) +{ + FieldEdit *fieldEdits = action->edits; + FieldList fields = action->requiredFields; + + while (fields != NULL) + { + const char *fldval = get_field_value (pr, oldPR, fields->ent, NULL, NULL); + if (value_is_empty (fldval)) + { + setError (err, CODE_INVALID_PR_CONTENTS, + "Required field %s missing from PR %s.", + complexFieldIndexToString (fields->ent), + field_value (pr, NUMBER (pr->database))); + return 1; + } + fields = fields->next; + } + + if (action->requireChangeReason && field_change_reason (pr, field) == NULL) + { + setError (err, CODE_INVALID_PR_CONTENTS, + "Edit of field %s requires a change reason.", + fieldDefForIndex (field)->name); + return 1; + } + + while (fieldEdits != NULL) + { + if (applyFieldEdit (pr, fieldEdits, err, params) != 0) + { + return 1; + } + fieldEdits = fieldEdits->next; + } + return 0; } static int @@ -88,6 +163,110 @@ addAuditEntryP (const DatabaseInfo datab } static int +applyChangeActions (PR *pr, PR *oldPR, FieldIndex field, + ChangeActions actions, ErrorDesc *err, + FormatNamedParameter *params) +{ + { + ChangeActions actionList = actions; + + while (actionList != NULL) + { + if (actionList->expr == NULL + || pr_matches_expr (pr, oldPR, actionList->expr, params)) + { + if (applyChangeAction (actionList, pr, oldPR, field, err, params)) + { + return 1; + } + } + actionList = actionList->next; + } + } + + if (field != InvalidFieldIndex && addAuditEntryP (pr->database, + field, actions)) + { + ChangeActions action = actions; + while (actions != NULL) + { + if (actions->addAuditTrail) + { + break; + } + actions = actions->next; + } + + if (action != NULL) + { + addAuditTrailEnt (pr, action->auditTrailFormat, params, err); + } + else + { + addAuditTrailEnt (pr, NULL, params, err); + } + } + + return 0; +} + +static int +processFieldChange (PR *pr, PR *oldPR, FieldIndex field, + ErrorDesc *err, const char *editUserEmailAddr, + const char *oldValue, const char *newValue) +{ + ChangeActions actions; + FormatNamedParameter *params = NULL; + int res; + + if (oldValue == NULL) + { + oldValue = ""; + } + if (newValue == NULL) + { + newValue = ""; + } + + if (fieldDefForIndex (field)->readonly) + { + setError (err, CODE_READONLY_FIELD, + newBadFieldEntry (field, NULL, NULL), + "Field %s is read-only: `%s'->`%s'", + fieldDefForIndex (field)->name, + oldValue, + newValue); + return 1; + } + + { + char *currDate = get_curr_date (); + params = allocateNamedParameter ("$CurrentDate", currDate, params); + free (currDate); + } + + params = allocateNamedParameter ("$FieldName", + fieldDefForIndex (field)->name, params); + params = allocateNamedParameter ("$OldValue", oldValue, params); + params = allocateNamedParameter ("$NewValue", newValue, params); + params = allocateNamedParameter ("$EditUserEmailAddr", editUserEmailAddr, + params); + { + const char *reason = field_change_reason (pr, field); + if (reason == NULL) + { + reason = ""; + } + params = allocateNamedParameter ("$ChangeReason", reason , params); + } + + actions = fieldDefForIndex (field)->changeActions; + res = applyChangeActions (pr, oldPR, field, actions, err, params); + freeFormatParameterList (params); + return res; +} + +static int processPRChanges (const char *editUserEmailAddr, PR *old_pr, PR *new_pr, ErrorDesc *err) { @@ -152,7 +331,14 @@ processPRChanges (const char *editUserEm if (fieldDefForIndex (field)->readonly) { fieldsChanged[x] = 0; - set_field (new_pr, field, old_value, err); + if (old_value == NULL) + { + unsetField (new_pr, field); + } + else + { + set_field (new_pr, field, old_value, err); + } } else { @@ -211,85 +397,42 @@ processPRChanges (const char *editUserEm /* Replace an existing PR with the same PR number as NEW_PR. EDITUSEREMAILADDR is the email address of the user that is performing the edit. */ -int -rewrite_pr (PR *new_pr, const char *editUserEmailAddr, ErrorDesc *err) +static int +rewrite_pr (PR *pr, PR *new_pr, const char *editUserEmailAddr, ErrorDesc *err) { const DatabaseInfo database = new_pr->database; FILE *prfile; - char *path = NULL; - char *old_path = NULL; + char *new_path = NULL; + char *old_path = NULL; char *bkup_path = NULL; - PR *old_pr = NULL; - int res = 1; newAuditTrailEntries = NULL; - if (field_value (new_pr, NUMBER (database)) == NULL) - { - free_pr (new_pr); - setError (err, CODE_INVALID_PR_CONTENTS, - "Missing PR number from new PR contents"); - return 0; - } - if (field_value (new_pr, CATEGORY (database)) == NULL) { - free_pr (new_pr); setError (err, CODE_INVALID_PR_CONTENTS, "Missing category from new PR contents"); return 0; } - if (! prExists (new_pr->database, field_value (new_pr, NUMBER (database)), - err)) - { - free_pr (new_pr); - return 0; - } - if (! isPrLocked (new_pr->database, field_value (new_pr, NUMBER (database)))) { setError (err, CODE_PR_NOT_LOCKED, "PR %s is not locked", field_value (new_pr, NUMBER (database))); - free_pr (new_pr); return 0; } - if (! check_pr (new_pr, err, 0)) + if (processPRChanges (editUserEmailAddr, pr, new_pr, err) != 0) { return 0; } - old_pr = replaceCurrentPRInIndex (new_pr); - - if (old_pr == NULL) - { - setError (err, CODE_NO_INDEX, "Unable to locate existing PR %s", - field_value (new_pr, NUMBER (database))); - free_pr (new_pr); - return 0; - } - - old_path = gen_pr_path (old_pr); - - /* set this to be the file to be saved. now called .old. */ - asprintf (&bkup_path, "%s.old", old_path); - - if (processPRChanges (editUserEmailAddr, old_pr, new_pr, err) != 0) - { - clearPRChain (database); - free_pr (old_pr); - free (bkup_path); - free (old_path); - return 0; - } - /* check to see if the category changes, and if so, write the new file out, and unlink the old one. */ - if (strcmp (field_value (old_pr, CATEGORY (database)), + if (strcmp (field_value (pr, CATEGORY (database)), field_value (new_pr, CATEGORY (database))) != 0) { char *dirPath; @@ -309,10 +452,6 @@ rewrite_pr (PR *new_pr, const char *edit "Error creating directory for category %s: %s", field_value (new_pr, CATEGORY (database)), strerror (errno)); - clearPRChain (database); - free_pr (old_pr); - free (bkup_path); - free (old_path); free (dirPath); return 0; } @@ -322,44 +461,32 @@ rewrite_pr (PR *new_pr, const char *edit setError (err, CODE_FILE_ERROR, "Directory for category %s does not exist", field_value (new_pr, CATEGORY (database))); - clearPRChain (database); - free_pr (old_pr); - free (bkup_path); - free (old_path); free (dirPath); return 0; } } free (dirPath); - path = gen_pr_path (new_pr); - } - else - { - path = gen_pr_path (old_pr); } + /* backup the current PR file */ + old_path = gen_pr_path (pr); + asprintf (&bkup_path, "%s.old", old_path); if (rename (old_path, bkup_path) < 0) { if (errno != EXDEV) { setError (err, CODE_FILE_ERROR, "Could not rename %s: %s", old_path, strerror (errno)); - clearPRChain (database); - free_pr (old_pr); free (bkup_path); free (old_path); - free (path); return 0; } if (copy_file (old_path, bkup_path) != 0) { setError (err, CODE_FILE_ERROR, "Could not copy %s to %s: %s", old_path, bkup_path, strerror (errno)); - clearPRChain (database); - free_pr (old_pr); free (bkup_path); free (old_path); - free (path); return 0; } @@ -369,16 +496,15 @@ rewrite_pr (PR *new_pr, const char *edit } /* Now build the file. */ - prfile = fopen (path, "w+"); + new_path = gen_pr_path (new_pr); + prfile = fopen (new_path, "w+"); if (prfile == (FILE *) NULL) { setError (err, CODE_FILE_ERROR, "Cannot write the PR to %s: %s", - path, strerror (errno)); - clearPRChain (database); - free_pr (old_pr); + new_path, strerror (errno)); free (bkup_path); free (old_path); - free (path); + free (new_path); return 0; } @@ -394,25 +520,28 @@ rewrite_pr (PR *new_pr, const char *edit if (fclose (prfile) == EOF) { setError (err, CODE_FILE_ERROR, "Error writing out PR %s: %s", - path, strerror (errno)); - clearPRChain (database); - free_pr (old_pr); + new_path, strerror (errno)); free (bkup_path); free (old_path); - free (path); + free (new_path); return 0; } unlink (bkup_path); /* unlink the old file, if it is in another category dir. */ - if (strcmp (field_value (old_pr, CATEGORY (database)), + if (strcmp (field_value (pr, CATEGORY (database)), field_value (new_pr, CATEGORY (database))) != 0) { unlink (old_path); } + free (bkup_path); + free (old_path); + free (new_path); + /* write out the new index. */ + replaceCurrentPRInIndex (pr, new_pr, err); if (writeIndex (database, err) != 0) { res = 0; @@ -421,16 +550,64 @@ rewrite_pr (PR *new_pr, const char *edit /* Send mail about the edit. */ if (newAuditTrailEntries != NULL) { - sendAuditMail (new_pr, old_pr, editUserEmailAddr, newAuditTrailEntries, + sendAuditMail (new_pr, pr, editUserEmailAddr, newAuditTrailEntries, err); free (newAuditTrailEntries); newAuditTrailEntries = NULL; } - free_pr (old_pr); - free (bkup_path); - free (old_path); - free (path); + return res; +} + + +int +replace_pr (const DatabaseInfo database, FILE *fp, + const char *editUserEmailAddr, ErrorDesc *err) +{ + int res; + const char *prnum = NULL; + PR *curr_pr; + PR *new_pr = allocPR (database); + + /* Read the message header in. */ + if (read_header (new_pr, fp) < 0) + { + setError (err, CODE_EOF_PR, "Couldn't read PR header"); + return 0; + } + + read_pr (new_pr, fp, 0); + + prnum = field_value (new_pr, NUMBER (database)); + if (prnum == NULL) + { + setError (err, CODE_INVALID_PR_CONTENTS, + "Missing PR number from new PR contents"); + return 0; + } + + if (! prExists (database, prnum, err)) + { + return 0; + } + + curr_pr = readPRWithNum (database, prnum, 0, err); + if (curr_pr == NULL) + { + setError (err, CODE_NO_INDEX, + "Unable to locate existing PR %s", prnum); + return 0; + } + + if (! check_pr (new_pr, err, 0)) + { + return 0; + } + + res = rewrite_pr (curr_pr, new_pr, editUserEmailAddr, err); + + free_pr (curr_pr); + free_pr (new_pr); return res; } @@ -815,17 +992,25 @@ edit_field (const DatabaseInfo database, char pid[32]; const char *oldfield; int res; - PR *pr; + PR *pr, *new_pr; if (! prExists (database, prnum, err)) { free (newcontents); + if (changeReason != NULL) + { + free (changeReason); + } return 0; } if (fieldIndex == InvalidFieldIndex) { setError (err, CODE_INVALID_FIELD_NAME, "Invalid field name"); free (newcontents); + if (changeReason != NULL) + { + free (changeReason); + } return 0; } if (requiresChangeReason (fieldDefForIndex (fieldIndex)) @@ -837,10 +1022,15 @@ edit_field (const DatabaseInfo database, free (newcontents); return 0; } + sprintf (pid, "%d", (int) getpid ()); if (lock_pr (database, prnum, "edit_field", pid, err) == 0) { free (newcontents); + if (changeReason != NULL) + { + free (changeReason); + } return 0; } @@ -849,7 +1039,6 @@ edit_field (const DatabaseInfo database, { unlock_pr (database, prnum, err); setError (err, CODE_UNREADABLE_PR, "Error reading PR %s.", prnum); - free_pr (pr); free (newcontents); if (changeReason != NULL) { @@ -876,36 +1065,58 @@ edit_field (const DatabaseInfo database, newcontents = totalVal; } + if (!validateFieldValue (fieldIndex, newcontents, err, 0)) + { + unlock_pr (database, prnum, err); + free (newcontents); + if (changeReason != NULL) + { + free (changeReason); + } + return 0; + } + + /* create a copy of the currently active pr so that we can modify it and + do subsequent validation allowing for us to bail out cleanly */ + new_pr = allocPR (database); + set_field (new_pr, NUMBER (database), prnum, err); + set_field (new_pr, CATEGORY (database), field_value (pr, CATEGORY (database)), + err); + fillInPR (new_pr, err); + /* set_field () verifies that the value is valid before doing the set. */ - if (set_field (pr, fieldIndex, newcontents, err) == 0) + /* set_field returns a boolean!!! */ + if (set_field (new_pr, fieldIndex, newcontents, err) == 0) { res = 0; - free_pr (pr); } else { if (changeReason != NULL) { - setFieldChangeReason (pr, fieldIndex, changeReason); + setFieldChangeReason (new_pr, fieldIndex, changeReason); free (changeReason); changeReason = NULL; } - res = rewrite_pr (pr, editUserEmailAddr, err); - } - - if (unlock_pr (database, prnum, err) == 0) - { - res = 0; + res = rewrite_pr (pr, new_pr, editUserEmailAddr, err); } + free_pr (pr); + free_pr (new_pr); free (newcontents); if (changeReason != NULL) - { + { free (changeReason); changeReason = NULL; + } + + if (unlock_pr (database, prnum, err) == 0) + { + res = 0; } + return res; } @@ -1029,262 +1240,33 @@ applyFieldEdit (PR *pr, FieldEdit *edit, return res; } -/* Add an entry to the AUDIT_TRAIL field. PARAMS are the format - parameters used when composing the entry. FMT is the format of the - audit-trail entry to add. */ - -static int -addAuditTrailEnt (PR *pr, QueryFormat *fmt, - FormatNamedParameter *params, ErrorDesc *err) -{ - char *newAuditString = NULL; - char *finalAuditString; - const char *t; - char *currDate = get_curr_date (); - - if (fmt == NULL) - { - fmt = getAuditTrailFormat (pr->database); - } - - /* Format the new audit entry. */ - process_format (NULL, &newAuditString, pr, NULL, fmt, "\n", params); - - /* Squirrel it away, because we'll need to mail it later. */ - append_string (&newAuditTrailEntries, newAuditString); - - /* Now append the formatted string to the audit-trail field. */ - t = field_value (pr, AUDIT_TRAIL (pr->database)); - if (t == NULL) - { - t = ""; - } - finalAuditString = xstrdup (t); - append_string (&finalAuditString, newAuditString); - set_field (pr, AUDIT_TRAIL (pr->database), finalAuditString, err); - - free (finalAuditString); - free (newAuditString); - free (currDate); - - return 0; -} - -static void -sendAuditMail (PR *pr, PR *oldPR, const char *editUserEmailAddr, - const char *newAuditTrailEntries, ErrorDesc *err) -{ - FormatNamedParameter *parms = NULL; - const DatabaseInfo database = pr->database; - - parms = allocateNamedParameter ("$EditUserEmailAddr", editUserEmailAddr, - parms); - parms = allocateNamedParameter ("$NewAuditTrail", newAuditTrailEntries, - parms); - - if (strcmp (field_value (pr, RESPONSIBLE (database)), - field_value (oldPR, RESPONSIBLE (database))) != 0) - { - parms - = allocateNamedParameter ("$OldResponsible", - field_value (oldPR, RESPONSIBLE (database)), - parms); - } - - composeMailMessage (pr, oldPR, "audit-mail", parms, NULL, err); - freeFormatParameterList (parms); -} - -static int -applyChangeAction (ChangeActions action, PR *pr, PR *oldPR, FieldIndex field, - ErrorDesc *err, FormatNamedParameter *params) -{ - FieldEdit *fieldEdits = action->edits; - FieldList fields = action->requiredFields; - - while (fields != NULL) - { - const char *fldval = get_field_value (pr, oldPR, fields->ent, NULL, NULL); - if (value_is_empty (fldval)) - { - setError (err, CODE_INVALID_PR_CONTENTS, - "Required field %s missing from PR %s.", - complexFieldIndexToString (fields->ent), - field_value (pr, NUMBER (pr->database))); - return 1; - } - fields = fields->next; - } - - if (action->requireChangeReason && field_change_reason (pr, field) == NULL) - { - setError (err, CODE_INVALID_PR_CONTENTS, - "Edit of field %s requires a change reason.", - fieldDefForIndex (field)->name); - return 1; - } - - while (fieldEdits != NULL) - { - if (applyFieldEdit (pr, fieldEdits, err, params) != 0) - { - return 1; - } - fieldEdits = fieldEdits->next; - } - return 0; -} - -static int -applyChangeActions (PR *pr, PR *oldPR, FieldIndex field, - ChangeActions actions, ErrorDesc *err, - FormatNamedParameter *params) -{ - { - ChangeActions actionList = actions; - - while (actionList != NULL) - { - if (actionList->expr == NULL - || pr_matches_expr (pr, oldPR, actionList->expr, params)) - { - if (applyChangeAction (actionList, pr, oldPR, field, err, params)) - { - return 1; - } - } - actionList = actionList->next; - } - } - - if (field != InvalidFieldIndex && addAuditEntryP (pr->database, - field, actions)) - { - ChangeActions action = actions; - while (actions != NULL) - { - if (actions->addAuditTrail) - { - break; - } - actions = actions->next; - } - - if (action != NULL) - { - addAuditTrailEnt (pr, action->auditTrailFormat, params, err); - } - else - { - addAuditTrailEnt (pr, NULL, params, err); - } - } - - return 0; -} - -static int -processFieldChange (PR *pr, PR *oldPR, FieldIndex field, - ErrorDesc *err, const char *editUserEmailAddr, - const char *oldValue, const char *newValue) -{ - ChangeActions actions; - FormatNamedParameter *params = NULL; - int res; - - if (oldValue == NULL) - { - oldValue = ""; - } - if (newValue == NULL) - { - newValue = ""; - } - - if (fieldDefForIndex (field)->readonly) - { - setError (err, CODE_READONLY_FIELD, - newBadFieldEntry (field, NULL, NULL), - "Field %s is read-only: `%s'->`%s'", - fieldDefForIndex (field)->name, - oldValue, - newValue); - return 1; - } - - { - char *currDate = get_curr_date (); - params = allocateNamedParameter ("$CurrentDate", currDate, params); - free (currDate); - } - - params = allocateNamedParameter ("$FieldName", - fieldDefForIndex (field)->name, params); - params = allocateNamedParameter ("$OldValue", oldValue, params); - params = allocateNamedParameter ("$NewValue", newValue, params); - params = allocateNamedParameter ("$EditUserEmailAddr", editUserEmailAddr, - params); - { - const char *reason = field_change_reason (pr, field); - if (reason == NULL) - { - reason = ""; - } - params = allocateNamedParameter ("$ChangeReason", reason , params); - } - - actions = fieldDefForIndex (field)->changeActions; - res = applyChangeActions (pr, oldPR, field, actions, err, params); - freeFormatParameterList (params); - return res; -} - /* Delete PR PRNUM. */ int deletePR (const DatabaseInfo database, const char *prnum, const char *editUserEmailAddr, ErrorDesc *err) { - char *path = lookup_pr_path (database, prnum, err); char pid[32]; FormatNamedParameter *parms = NULL; - - if (path == NULL) - { - return -1; - } + short delete_status; if (client_lock_gnats (database, err)) { - free (path); return -4; } if (lock_pr (database, prnum, GNATS_USER, pid, err) == 0) { client_unlock_gnats (); - free (path); return -4; } - if (removePRFromIndex (database, prnum, err)) - { - unlock_pr (database, prnum, err); - client_unlock_gnats (); - free (path); - return -5; - } - - if (unlink (path)) + if ((delete_status = pr_delete (database, prnum, err)) < 0) { - setError (err, CODE_FILE_ERROR, "Unable to unlink file %s\n", path); unlock_pr (database, prnum, err); client_unlock_gnats (); - free (path); - return -6; + return delete_status; } - free (path); - if (unlock_pr (database, prnum, err) == 0) { client_unlock_gnats (); Index: file-pr.c =================================================================== RCS file: /cvsroot/gnats/gnats/gnats/file-pr.c,v retrieving revision 1.54 diff -u -p -r1.54 file-pr.c --- file-pr.c 26 Oct 2003 07:33:26 -0000 1.54 +++ file-pr.c 21 Feb 2005 12:19:39 -0000 @@ -118,13 +118,14 @@ createNewPRFile (PR *pr, int flag_autocr } } - if (submitter->admFields[SubmitterAdmRtime][0] != '\0') + if (submitter->admFields[SubmitterAdmRtime] == NULL || + submitter->admFields[SubmitterAdmRtime][0] == '\0') { - submitter_rtime = atoi (submitter->admFields[SubmitterAdmRtime]); + submitter_rtime = -1; } else { - submitter_rtime = -1; + submitter_rtime = atoi (submitter->admFields[SubmitterAdmRtime]); } /* If originator wasn't set, try to get the value from the From mail Index: gnats.h =================================================================== RCS file: /cvsroot/gnats/gnats/gnats/gnats.h,v retrieving revision 1.53 diff -u -p -r1.53 gnats.h --- gnats.h 30 Aug 2003 07:58:14 -0000 1.53 +++ gnats.h 21 Feb 2005 12:19:39 -0000 @@ -315,8 +315,6 @@ extern int check_pr (PR *pr, int initial_entry); extern int replace_pr (const DatabaseInfo database, FILE *infile, const char *editUserEmailAddr, ErrorDesc *err); -extern int rewrite_pr (PR *pr, const char *editUserEmailAddr, - ErrorDesc *err); extern int lock_pr (const DatabaseInfo database, const char *filename, const char *username, const char *processid, ErrorDesc *error); Index: gnatsd.c =================================================================== RCS file: /cvsroot/gnats/gnats/gnats/gnatsd.c,v retrieving revision 1.49 diff -u -p -r1.49 gnatsd.c --- gnatsd.c 30 Nov 2004 15:43:42 -0000 1.49 +++ gnatsd.c 21 Feb 2005 12:19:39 -0000 @@ -478,7 +478,7 @@ findUserAccessLevel (const char *file, c requested database. */ const char *l2 = ent->admFields[3]; - if (! strlen(l2)) + if (l2 == NULL || ! strlen(l2)) found = 1; while (l2 != NULL && ! found) Index: index.c =================================================================== RCS file: /cvsroot/gnats/gnats/gnats/index.c,v retrieving revision 1.40 diff -u -p -r1.40 index.c --- index.c 1 Dec 2001 22:52:12 -0000 1.40 +++ index.c 21 Feb 2005 12:19:40 -0000 @@ -61,6 +61,7 @@ struct indexEntry char *buf; char **fields; PR *nextPR; + PR *prevPR; }; static PR *nextIndexEntryBinary (IndexFileDesc fp); @@ -234,6 +235,7 @@ nextIndexEntry (IndexFileDesc fp) p = res->index; seplen = strlen (fp->desc->separator); + p->prevPR = NULL; p->nextPR = NULL; p->fields = (char **) xmalloc (sizeof (char *) * get_num_fields (fp->desc->database)); @@ -326,6 +328,7 @@ nextIndexEntryBinary (IndexFileDesc fp) res = allocPR (fp->desc->database); indexEnt = res->index; + indexEnt->prevPR = NULL; indexEnt->nextPR = NULL; indexEnt->fields = (char **) xmalloc (sizeof (char *) @@ -573,6 +576,7 @@ getIndex (IndexDesc indexDesc, ErrorDesc } else { + i->index->prevPR = end_chain; end_chain->index->nextPR = i; end_chain = i; } @@ -662,6 +666,7 @@ allocIndex (PR *pr) pr->index = (Index *) xmalloc (sizeof (Index)); pr->index->buf = NULL; pr->index->fields = NULL; + pr->index->prevPR = NULL; pr->index->nextPR = NULL; } @@ -1115,34 +1120,57 @@ clearPRChain (const DatabaseInfo databas } } -/* Find the PR in the current index with the same PR number as NEW_PR, - and replace it with NEW_PR. The old PR is returned, or a NULL - pointer if a PR with the same number is not found. */ +/* Replace a PR in the index with a copy of a new PR. + By generating a copy of the new PR, we allow for the new PR + to be free'd without harming the index pr chain. */ -PR * -replaceCurrentPRInIndex (PR *new_pr) +int +replaceCurrentPRInIndex (PR *curr_pr, PR *new_pr, ErrorDesc *err) { - ErrorDesc err; - IndexDesc indexDesc = getIndexDesc (new_pr->database); - PR **currptr = &(indexDesc->prChain); - const char *new_pr_num = field_value (new_pr, NUMBER (new_pr->database)); + PR *pr_replace, *pr_copy; + const DatabaseInfo database = curr_pr->database; + const char *new_pr_num = field_value (new_pr, NUMBER (database)); + const char *curr_pr_num = field_value (curr_pr, NUMBER (database)); + + if (strcmp (curr_pr_num, new_pr_num) != 0) + { + return 0; + } + + /* create a copy of the new pr to be put into the index pr chain */ + pr_copy = allocPR (database); + set_field (pr_copy, NUMBER (curr_pr->database), new_pr_num, err); + set_field (pr_copy, CATEGORY (database), + field_value (new_pr, CATEGORY (database)), err); + setPrevPR (pr_copy, curr_pr->index->prevPR); + setNextPR (pr_copy, curr_pr->index->nextPR); + fillInPR (pr_copy, err); + + /* put the copy of new_pr into place in the index pr chain... */ + + if (curr_pr->index->prevPR == NULL) + { + /* the pr to be replaced is at the head of the pr chain */ + IndexDesc indexDesc = getIndexDesc (database); + free_pr (indexDesc->prChain); + indexDesc->prChain = pr_copy; + return 1; + } - getFirstPR (new_pr->database, &err); + /* save a pointer to the pr to be replaced so that it can later be free'd */ + pr_replace = getNextPR (curr_pr->index->prevPR); - while ((*currptr) != NULL) + setNextPR (curr_pr->index->prevPR, pr_copy); + + if (curr_pr->index->nextPR != NULL) { - if (strcmp (field_value (*currptr, NUMBER (new_pr->database)), - new_pr_num) == 0) - { - PR *old_pr = (*currptr); - new_pr->index->nextPR = old_pr->index->nextPR; - old_pr->index->nextPR = NULL; - (*currptr) = new_pr; - return old_pr; - } - currptr = &((*currptr)->index->nextPR); + /* the pr to be replaced is not at the end of the pr chain */ + setPrevPR (curr_pr->index->nextPR, pr_copy); } - return NULL; + + free_pr (pr_replace); + + return 1; } /* Remove PR PRNUM from the index for the current database, and rewrite the @@ -1196,6 +1224,24 @@ getNextPR (PR *pr) return pr->index->nextPR; } +void +setNextPR (PR *pr, PR *next_pr) +{ + pr->index->nextPR = next_pr; +} + +PR * +getPrevPR (PR *pr) +{ + return pr->index->prevPR; +} + +void +setPrevPR (PR *pr, PR *prev_pr) +{ + pr->index->prevPR = prev_pr; +} + int indexIsBinary (const DatabaseInfo database) { Index: index.h =================================================================== RCS file: /cvsroot/gnats/gnats/gnats/index.h,v retrieving revision 1.17 diff -u -p -r1.17 index.h --- index.h 15 Apr 2001 18:11:51 -0000 1.17 +++ index.h 21 Feb 2005 12:19:40 -0000 @@ -24,7 +24,10 @@ extern int checkPRChain (const DatabaseI extern PR *getFirstPR (const DatabaseInfo database, ErrorDesc *err); +extern PR *getPrevPR (PR *pr); extern PR *getNextPR (PR *pr); +extern void setPrevPR (PR *pr, PR *prev_pr); +extern void setNextPR (PR *pr, PR *next_pr); extern void freePRIndex (PR *pr); @@ -46,7 +49,7 @@ extern int addToIndex (PR *pr, ErrorDesc extern char *createIndexEntry (PR *pr, size_t *entLen); extern char *createIndexEntryBinary (PR *pr, size_t *entLen); -extern PR *replaceCurrentPRInIndex (PR *newPR); +extern int replaceCurrentPRInIndex (PR *curr_pr, PR *newPR, ErrorDesc *err); extern char *indexValue (PR *pr, FieldIndex field); Index: mail.c =================================================================== RCS file: /cvsroot/gnats/gnats/gnats/mail.c,v retrieving revision 1.22 diff -u -p -r1.22 mail.c --- mail.c 30 Nov 2004 15:43:50 -0000 1.22 +++ mail.c 21 Feb 2005 12:19:40 -0000 @@ -65,9 +65,13 @@ get_responsible_address (const DatabaseI passwd file. */ if (res != NULL) { - if (res->admFields[ResponsibleAdmAlias] == '\0') + if (res->admFields[ResponsibleAdmAlias] == NULL || + res->admFields[ResponsibleAdmAlias] == '\0') { - free (res->admFields[ResponsibleAdmAlias]); + if (res->admFields[ResponsibleAdmAlias] != NULL) + { + free (res->admFields[ResponsibleAdmAlias]); + } res->admFields[ResponsibleAdmAlias] = xstrdup (res->admFields[ResponsibleAdmKey]); } Index: pr.c =================================================================== RCS file: /cvsroot/gnats/gnats/gnats/pr.c,v retrieving revision 1.64 diff -u -p -r1.64 pr.c --- pr.c 25 Nov 2002 13:58:33 -0000 1.64 +++ pr.c 21 Feb 2005 12:19:40 -0000 @@ -154,7 +154,6 @@ allocPR (const DatabaseInfo database) } /* Get the PR at PATH. PR is the PR entry to be filled in. - If PRUNE is non-zero, don't read any multitext fields. */ static int get_pr (PR *pr, const char *path, int prune) @@ -211,6 +210,33 @@ fillInPR (PR *pr, ErrorDesc *err) } } +static PR * +get_pr_from_index (const DatabaseInfo database, const char *prnum, + ErrorDesc *err) +{ + PR *pr = getFirstPR (database, err); + + /* If they gave it to us with the category, remove it. */ + if (( strrchr (prnum, '/')) != NULL) + { + prnum = strrchr (prnum, '/') + 1; + } + + while (pr != NULL && strcmp (prnum, field_value (pr, NUMBER (database))) != 0) + { + pr = getNextPR (pr); + } + + if (pr == NULL) + { + setError (err, CODE_NONEXISTENT_PR, + "No PR %s listed in the index.", prnum); + return NULL; + } + + return pr; +} + /* Initializes PR for reading. Each line of the PR should be passed in via addLineToPR (). */ @@ -1348,29 +1374,16 @@ free_pr (PR *pr) } static char * -get_pr_path (const DatabaseInfo database,const char *prnum, ErrorDesc *err) +get_pr_path (const DatabaseInfo database, PR *pr, const char *prnum, + ErrorDesc *err) { char *path = NULL; const char *category = NULL; - PR *curr_pr_chain = getFirstPR (database, err); char *categoryFromIndex = NULL; - if (curr_pr_chain != NULL) + if (pr != NULL) { - PR *j; - for (j = curr_pr_chain ; j != NULL ; j = getNextPR (j)) - { - if (strcmp (prnum, field_value (j, NUMBER (database))) == 0) - { - category = field_value (j, CATEGORY (database)); - break; - } - } - if (category == NULL) - { - setError (err, CODE_NONEXISTENT_PR, - "No PR %s listed in the index.", prnum); - } + category = field_value (pr, CATEGORY (database)); } else { @@ -1391,9 +1404,10 @@ get_pr_path (const DatabaseInfo database } int -prExists (const DatabaseInfo database, const char *prID, ErrorDesc *err) +prExists (const DatabaseInfo database, const char *prnum, ErrorDesc *err) { - char *path = get_pr_path (database, prID, err); + PR *pr = get_pr_from_index (database, prnum, err); + char *path = get_pr_path (database, pr, prnum, err); int res = 0; if (path != NULL) { @@ -1407,48 +1421,85 @@ prExists (const DatabaseInfo database, c return res; } -char * -lookup_pr_path (const DatabaseInfo database, const char *prnum, ErrorDesc *err) +static bool +pr_file_readable (const char *path, ErrorDesc *err) { - char *path; FILE *fp; - /* If they gave it to us with the category, remove it. */ - if (( strrchr (prnum, '/')) != NULL) + if (path == NULL) { - prnum = strrchr (prnum, '/') + 1; + return FALSE; } - path = get_pr_path (database, prnum, err); - if ((path == NULL) || ((fp = fopen (path, "r")) == NULL)) + + if ((fp = fopen (path, "r")) == NULL) { - if (path != NULL) - { - setError (err, CODE_FILE_ERROR, "Can't open file `%s'", path); - } - return NULL; + setError (err, CODE_FILE_ERROR, "Can't open file `%s'", path); + return FALSE; } fclose (fp); - return path; + return TRUE; } PR * -readPRWithNum (const DatabaseInfo database, const char *prID, +readPRWithNum (const DatabaseInfo database, const char *prnum, int prune, ErrorDesc *err) { PR *pr = NULL; - char *path = lookup_pr_path (database, prID, err); + PR *index_pr = get_pr_from_index (database, prnum, err); + char *path = get_pr_path (database, index_pr, prnum, err); if (path != NULL) { - pr = allocPR (database); - if (get_pr (pr, path, prune) == 0) + if (pr_file_readable (path, err)) + { + pr = allocPR (database); + setPrevPR (pr, getPrevPR (index_pr)); + setNextPR (pr, getNextPR (index_pr)); + if (get_pr (pr, path, prune) == 0) + { + free_pr (pr); + pr = NULL; + } + } + free (path); + } + return pr; +} + +int +pr_delete (const DatabaseInfo database, const char *prnum, ErrorDesc *err) +{ + PR *pr; + char *path = NULL; + + pr = get_pr_from_index (database, prnum, err); + path = get_pr_path (database, pr, prnum, err); + + if (path == NULL || !pr_file_readable (path, err)) + { + if (path != NULL) { - free_pr (pr); - pr = NULL; + free (path); } + return -1; } - return pr; + + if (removePRFromIndex (database, prnum, err)) + { + free (path); + return -5; + } + + if (unlink (path)) + { + setError (err, CODE_FILE_ERROR, "Unable to unlink file %s\n", path); + free (path); + return -6; + } + + free (path); + return 1; } void Index: pr.h =================================================================== RCS file: /cvsroot/gnats/gnats/gnats/pr.h,v retrieving revision 1.38 diff -u -p -r1.38 pr.h --- pr.h 15 Apr 2001 18:11:51 -0000 1.38 +++ pr.h 21 Feb 2005 12:19:40 -0000 @@ -131,8 +131,8 @@ extern int fconfigParse (DatabaseInfo da extern int createPrFile (PR *pr, const char *filename, int force, ErrorDesc *err); -extern char *lookup_pr_path (const DatabaseInfo database, - const char *prnum, ErrorDesc *); +extern int pr_delete (const DatabaseInfo database, const char *prnum, + ErrorDesc *err); extern int prExists (const DatabaseInfo database, const char *prID, ErrorDesc *err);