Yes please. As noted in older thread that XXX block in rcs.c produced side 
effects with cvs annotate.
https://marc.info/?l=openbsd-tech&m=144757775319206&w=2


On Wed, Jun 22, 2016 at 05:20:01PM +0200, Joris Vink wrote:
> On Wed, Jun 22, 2016 at 09:07:03AM -0600, Todd C. Miller wrote:
> > On Wed, 22 Jun 2016 12:21:56 +0200, Joris Vink wrote:
> > > Index: rcs.c
> > > ===================================================================
> > > RCS file: /cvs/src/usr.bin/cvs/rcs.c,v
> > > retrieving revision 1.313
> > > diff -u -p -r1.313 rcs.c
> > > --- rcs.c 5 Nov 2015 09:48:21 -0000       1.313
> > > +++ rcs.c 22 Jun 2016 09:52:04 -0000
> > > @@ -1796,17 +1796,13 @@ rcs_rev_getlines(RCSFILE *rfp, RCSNUM *f
> > >  
> > >  again:
> > >   for (;;) {
> > > +         if (rdp == NULL)
> > > +                 break;
> > 
> > Wouldn't this be easier to read as:
> > 
> >     while (rdp != NULL) {
> 
> Yes, updated diff below.
> 
> .joris
> 
> Index: rcs.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/cvs/rcs.c,v
> retrieving revision 1.313
> diff -u -p -r1.313 rcs.c
> --- rcs.c     5 Nov 2015 09:48:21 -0000       1.313
> +++ rcs.c     22 Jun 2016 15:13:14 -0000
> @@ -1795,18 +1795,11 @@ rcs_rev_getlines(RCSFILE *rfp, RCSNUM *f
>               goto done;
>  
>  again:
> -     for (;;) {
> +     while (rdp != NULL) {
>               if (rdp->rd_next->rn_len != 0) {
>                       trdp = rcs_findrev(rfp, rdp->rd_next);
>                       if (trdp == NULL)
>                               fatal("failed to grab next revision");
> -             } else {
> -                     /*
> -                      * XXX Fail, although the caller does not always do the
> -                      * right thing (eg cvs diff when the tree is ahead of
> -                      * the repository).
> -                      */
> -                     break;
>               }
>  
>               if (rdp->rd_tlen == 0) {
> @@ -1857,7 +1850,7 @@ again:
>       }
>  
>  next:
> -     if (!rcsnum_differ(rdp->rd_num, frev))
> +     if (rdp == NULL || !rcsnum_differ(rdp->rd_num, frev))
>               done = 1;
>  
>       if (RCSNUM_ISBRANCHREV(frev) && done != 1) {
> @@ -2045,6 +2038,7 @@ rcs_rev_getbuf(RCSFILE *rfp, RCSNUM *rev
>       struct rcs_delta *rdp;
>       struct rcs_lines *lines;
>       struct rcs_line *lp, *nlp;
> +     char version[RCSNUM_MAXSTR];
>       BUF *bp;
>  
>       rdp = NULL;
> @@ -2057,8 +2051,12 @@ rcs_rev_getbuf(RCSFILE *rfp, RCSNUM *rev
>               expmode = rcs_kwexp_get(rfp);
>  
>               if (!(expmode & RCS_KWEXP_NONE)) {
> -                     if ((rdp = rcs_findrev(rfp, rev)) == NULL)
> -                             fatal("could not fetch revision");
> +                     if ((rdp = rcs_findrev(rfp, rev)) == NULL) {
> +                             rcsnum_tostr(rev, version, sizeof(version));
> +                             fatal("could not find desired version %s in %s",
> +                                 version, rfp->rf_path);
> +                     }
> +
>                       expand = 1;
>               }
>       }
> 

Reply via email to