[Cvs-dev] Re: [Cvs-cvs] ccvs/src ChangeLog rcs.c

[Derek R. Price]

Larry,

I restored the error handling you removed and repaired it.  Without this
change, memory leaked on non-fatal errors.

Cheers,

Derek


Larry Jones wrote:
> CVSROOT:      /cvsroot/cvs
> Module name:  ccvs
> Changes by:   Larry Jones <scjones>   06/09/06 22:10:15
> 
> Modified files:
>       src            : ChangeLog rcs.c 
> 
> Log message:
>       * rcs.c (apply_rcs_changes): Improve linked list handling.  Remove
>       unused variables and unreachable error handling code.  Avoid unneeded
>       dynamic allocation of temp linevector.  Minor stylistic code clean up.
> 
> CVSWeb URLs:
> http://cvs.savannah.gnu.org/viewcvs/ccvs/src/ChangeLog?cvsroot=cvs&r1=1.3493&r2=1.3494
> http://cvs.savannah.gnu.org/viewcvs/ccvs/src/rcs.c?cvsroot=cvs&r1=1.379&r2=1.380
> 
> Patches:
> Index: ChangeLog
> ===================================================================
> RCS file: /cvsroot/cvs/ccvs/src/ChangeLog,v
> retrieving revision 1.3493
> retrieving revision 1.3494
> diff -u -b -r1.3493 -r1.3494
> --- ChangeLog 6 Sep 2006 19:42:36 -0000       1.3493
> +++ ChangeLog 6 Sep 2006 22:10:15 -0000       1.3494
> @@ -1,3 +1,9 @@
> +2006-09-06  Larry Jones  <address@hidden>
> +
> +     * rcs.c (apply_rcs_changes): Improve linked list handling.  Remove
> +     unused variables and unreachable error handling code.  Avoid unneeded
> +     dynamic allocation of temp linevector.  Minor stylistic code clean up.
> +
>  2006-09-06  Derek Price  <address@hidden>
>  
>       [bug #17560]
> 
> Index: rcs.c
> ===================================================================
> RCS file: /cvsroot/cvs/ccvs/src/rcs.c,v
> retrieving revision 1.379
> retrieving revision 1.380
> diff -u -b -r1.379 -r1.380
> --- rcs.c     6 Sep 2006 19:42:36 -0000       1.379
> +++ rcs.c     6 Sep 2006 22:10:15 -0000       1.380
> @@ -7411,7 +7411,7 @@
>   *   program to exit.
>   */
>  static int
> -apply_rcs_changes (struct linevector *lines, const char *diffbuf,
> +apply_rcs_changes (struct linevector *orig_lines, const char *diffbuf,
>                  size_t difflen, const char *name, RCSVers *addvers,
>                  RCSVers *delvers)
>  {
> @@ -7430,18 +7430,14 @@
>       struct deltafrag *next;
>      };
>      struct deltafrag *dfhead;
> -    struct deltafrag *dftail;
> +    struct deltafrag **dftail;
>      struct deltafrag *df;
> -    int err;
>      unsigned long numlines, lastmodline, offset;
> -    struct linevector *orig_lines = lines;
> +    struct linevector lines;
>  
> -    /* The only reason to set dftail to NULL is to appease GCC.  GCC prints a
> -     * warning about its potentially being used uninitialized, even though 
> the
> -     * logic here prevents that.
> -     */
> -    dfhead = dftail = NULL;
> -    numlines = lines->nlines; /* start with init # of lines */
> +    dfhead = NULL;
> +    dftail = &dfhead;
> +    numlines = orig_lines->nlines; /* start with init # of lines */
>      for (p = diffbuf; p != NULL && p < diffbuf + difflen; )
>      {
>       op = *p++;
> @@ -7450,13 +7446,8 @@
>              of op determines the syntax.  */
>           error (1, 0, "unrecognized operation '\\x%x' in %s",
>                  op, name);
> -     df = xmalloc (sizeof (struct deltafrag));
> -     df->next = NULL;
> -     if (dfhead == NULL)
> -         dfhead = df;
> -     else
> -         dftail->next = df;
> -     dftail = df;
> +     *dftail = df = xmalloc (sizeof *df);
> +     *(dftail = &df->next) = NULL;
>  
>       df->pos = strtoul (p, (char **) &q, 10);
>  
> @@ -7514,10 +7505,8 @@
>  
>      /* New temp data structure to hold new org before
>         copy back into original structure. */
> -    lines = xmalloc (sizeof (struct linevector));
> -    lines->nlines = lines->lines_alloced = numlines;
> -    lines->vector = xnmalloc (numlines, sizeof (*lines->vector));
> -
> +    lines.nlines = lines.lines_alloced = numlines;
> +    lines.vector = xnmalloc (numlines, sizeof *lines.vector);
>  
>      /* We changed the list order to first to last -- so the
>         list never gets larger than the size numlines. */
> @@ -7527,29 +7516,20 @@
>         between new and original structure */
>      offset = 0; 
>  
> -    err = 0;
> -    for (df = dfhead; df != NULL;)
> +    for (df = dfhead; df != NULL; )
>      {
>       unsigned int ln;
> +     unsigned long deltaend;
>  
>       /* Here we need to get to the line where the next insert will
>          begin which is <df->pos> we will fill up to df->pos with
>          original items. */
> -     unsigned long deltaend;
> -
> -     /* Once an error is encountered, just free the rest of the list and
> -      * return.
> -      */
> -     if (!err)
> -     {
> -         for (deltaend = df->pos - offset;
> -              lastmodline < deltaend;
> -              lastmodline++)
> +     for (deltaend = df->pos - offset; lastmodline < deltaend; lastmodline++)
>           {
>               /* we need to copy from the orig structure into new one */
> -             lines->vector[lastmodline] =
> +         lines.vector[lastmodline] =
>                   orig_lines->vector[lastmodline + offset];
> -             lines->vector[lastmodline]->refcount++;
> +         lines.vector[lastmodline]->refcount++;
>           }
>  
>           switch (df->type)
> @@ -7561,12 +7541,10 @@
>                   struct line *q;
>                   int nextline_newline;
>                   size_t nextline_len;
> -                 int online;
>           
>                   textend = df->new_lines + df->len;
>                   nextline_newline = 0;
>                   nextline_text = df->new_lines;
> -                 online = 0; /* which line we are currently adding */
>                   for (p = df->new_lines; p < textend; ++p)
>                   {
>                       if (*p == '\n')
> @@ -7581,14 +7559,14 @@
>                           }
>  
>                           nextline_len = p - nextline_text;
> -                         q = xmalloc (sizeof (struct line) + nextline_len);
> +                     q = xmalloc (sizeof *q + nextline_len);
>                           q->vers = addvers;
> -                         q->text = (char *)q + sizeof (struct line);
> +                     q->text = (char *)(q + 1);
>                           q->len = nextline_len;
>                           q->has_newline = nextline_newline;
>                           q->refcount = 1;
>                           memcpy (q->text, nextline_text, nextline_len);
> -                         lines->vector[lastmodline++] = q;
> +                     lines.vector[lastmodline++] = q;
>                           offset--;
>               
>                           nextline_text = (char *)p + 1;
> @@ -7596,20 +7574,20 @@
>                       }
>                   }
>                   nextline_len = p - nextline_text;
> -                 q = xmalloc (sizeof (struct line) + nextline_len);
> +             q = xmalloc (sizeof *q + nextline_len);
>                   q->vers = addvers;
> -                 q->text = (char *)q + sizeof (struct line);
> +             q->text = (char *)(q + 1);
>                   q->len = nextline_len;
>                   q->has_newline = nextline_newline;
>                   q->refcount = 1;
>                   memcpy (q->text, nextline_text, nextline_len);
> -                 lines->vector[lastmodline++] = q;
> +             lines.vector[lastmodline++] = q;
>  
>                   /* For each line we add the offset between the #'s
> -                    increases. */
> +                decreases. */
>                   offset--;
> -             }
>               break;
> +         }
>  
>           case FRAG_DELETE:
>               /* we are removing this many lines from the source. */
> @@ -7629,43 +7607,29 @@
>                   }
>               break;
>           }
> -     }
>  
>       df = df->next;
>       free (dfhead);
>       dfhead = df;
>      }
>  
> -    if (err)
> -    {
> -     /* No reason to try and move a half-mutated and known invalid
> -      * text into the output buffer.
> -      */
> -     linevector_free (lines);
> -    }
> -    else
> -    {
>       /* add the rest of the remaining lines to the data vector */
>       for (; lastmodline < numlines; lastmodline++)
>       {
>           /* we need to copy from the orig structure into new one */
> -         lines->vector[lastmodline] = orig_lines->vector[lastmodline
> -                                                         + offset];
> -         lines->vector[lastmodline]->refcount++;
> +     lines.vector[lastmodline] = orig_lines->vector[lastmodline + offset];
> +     lines.vector[lastmodline]->refcount++;
>       }
>  
>       /* Move the lines vector to the original structure for output,
>        * first deleting the old.
>        */
>       linevector_free (orig_lines);
> -     orig_lines->vector = lines->vector;
> +    orig_lines->vector = lines.vector;
>       orig_lines->lines_alloced = numlines;
> -     orig_lines->nlines = lines->nlines;
> -    }
> +    orig_lines->nlines = lines.nlines;
>  
> -    free (lines);            /* we don't need it any longer */
> -
> -    return !err;
> +    return 1;
>  }
>  
>  
> 
> 
> _______________________________________________
> Cvs-cvs mailing list
> address@hidden
> http://lists.nongnu.org/mailman/listinfo/cvs-cvs


-- 
Derek R. Price
CVS Solutions Architect
Get CVS support at Ximbiot <http://ximbiot.com>!
v: +1 248.835.1260
f: +1 248.835.1263
<mailto:address@hidden>