From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8456 invoked from network); 21 Feb 2005 12:41:38 -0000 Received: from unknown (HELO lists.gnu.org) (199.232.76.165) by sourceware.org with SMTP; 21 Feb 2005 12:41:38 -0000 Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1D3D6D-0003wN-4F for listarch-gnats-devel@sources.redhat.com; Mon, 21 Feb 2005 07:55:41 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1D3D64-0003vo-7k for help-gnats@gnu.org; Mon, 21 Feb 2005 07:55:32 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1D3D5y-0003uX-P1 for help-gnats@gnu.org; Mon, 21 Feb 2005 07:55:31 -0500 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1D3D5y-0003u8-IS for help-gnats@gnu.org; Mon, 21 Feb 2005 07:55:26 -0500 Received: from [216.27.182.174] (helo=wattes.org) by monty-python.gnu.org with esmtp (Exim 4.34) id 1D3Cpa-0006Wt-AP for help-gnats@gnu.org; Mon, 21 Feb 2005 07:38:32 -0500 Received: from [10.64.209.22] ([10.64.209.22]) by wattes.org (8.11.6/8.11.6) with ESMTP id j1LCcJc03791; Mon, 21 Feb 2005 04:38:20 -0800 Message-ID: <4219D63F.4060909@wattes.org> Date: Mon, 21 Feb 2005 12:41:00 -0000 From: Mel Hatzis User-Agent: Mozilla Thunderbird 1.0 (Macintosh/20041206) X-Accept-Language: en-us, en MIME-Version: 1.0 To: Chad Walstrom , help-gnats@gnu.org Content-Type: multipart/mixed; boundary="------------000301040807040306050303" Cc: Subject: patch #1 - towards a generic backend datastore X-BeenThere: help-gnats@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: General discussion about GNU GNATS List-Archive: List-Post: List-Help: List-Subscribe: , Sender: help-gnats-bounces+listarch-gnats-devel=sources.redhat.com@gnu.org Errors-To: help-gnats-bounces+listarch-gnats-devel=sources.redhat.com@gnu.org X-SW-Source: 2005-q1/txt/msg00034.txt.bz2 This is a multi-part message in MIME format. --------------000301040807040306050303 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-length: 1034 Please review the attached patch which contains a number of bug fixes in addition to modifications intended to restructure the code to support a generalized backend datastore. I have developed a copy of GNATS (which I'm calling gnats-dbi) which utilizes a backend datastore api and can be configured with either a flat-file or Oracle backend. Anyone interested can download a copy from http://professional.juniper.net/gnats/. This patch is the first of many I plan to submit to merge my changes back into the official GNATS project and thereby enhance it with the ability to have multiple backends. This first patch is rather large. Hopefully, most of the subsequent ones will be easier to digest. The primary goal in this patch was to begin isolating the index logic from the PR edit functionality. A number of performance improvements have been made in the process. Note that I started developing and testing primarily on my Mac and this exposed a few memory related issues which I decided to fix in the process. -- Mel Hatzis --------------000301040807040306050303 Content-Type: text/plain; x-mac-type="0"; x-mac-creator="0"; name="patch.txt" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="patch.txt" Content-length: 42498 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); --------------000301040807040306050303 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 140 _______________________________________________ Help-gnats mailing list Help-gnats@gnu.org http://lists.gnu.org/mailman/listinfo/help-gnats --------------000301040807040306050303--