public inbox for gnats-devel@sourceware.org
 help / color / mirror / Atom feed
From: Mel Hatzis <mel@wattes.org>
To: Chad Walstrom <chewie@wookimus.net>
Cc: help-gnats@gnu.org
Subject: Re: patch #1 - towards a generic backend datastore
Date: Wed, 23 Feb 2005 07:24:00 -0000	[thread overview]
Message-ID: <421C2C84.3090509@wattes.org> (raw)
In-Reply-To: <4219D63F.4060909@wattes.org>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

No one has responded to my patch thus far. I plan on committing
the changes Thursday evening (PST) provided no one speaks up in
the meantime.

- --
Mel Hatzis

Mel Hatzis wrote:
> 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
> 
> 
> ------------------------------------------------------------------------
> 
> 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  <hatzis@wattes.org>
> +
> +	* 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  <chewie@wookimus.net>
>  
>  	* 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);
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Help-gnats mailing list
> Help-gnats@gnu.org
> http://lists.gnu.org/mailman/listinfo/help-gnats

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (Darwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD4DBQFCHCyDNF74HmYqaSERArPiAJYwb/hMKgGhjwqBZHmJ+8IRs8/iAJ48X77T
I5u/r6gE77UTGDOFsarTsg==
=6hH6
-----END PGP SIGNATURE-----


_______________________________________________
Help-gnats mailing list
Help-gnats@gnu.org
http://lists.gnu.org/mailman/listinfo/help-gnats

  reply	other threads:[~2005-02-23  7:24 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-02-21 12:41 Mel Hatzis
2005-02-23  7:24 ` Mel Hatzis [this message]
2005-02-24  0:10   ` Chad Walstrom
2005-02-24  7:06     ` Mel Hatzis
2005-02-24  8:06       ` Mike M. Volokhov
2005-02-24  8:53         ` Mel Hatzis
2005-02-24 11:15           ` Mark D. Baushke
2005-02-24 11:35           ` Mike M. Volokhov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=421C2C84.3090509@wattes.org \
    --to=mel@wattes.org \
    --cc=chewie@wookimus.net \
    --cc=help-gnats@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).