On Fri, May 09, 2014 at 06:01:52PM +0200, J??r??mie Courr??ges-Anglas wrote:
> Fritjof Bornebusch <[email protected]> writes:
> 
> > Hi tech,
> 
> Hi,
> 
> > if I compile rcs, gcc prints a few warnings like this:
> > - comparison between signed and unsigned
> > - signed and unsigned type in conditional expression
> >
> > I'm not quite sure if the typecasts are at the correct place, but these 
> > diffs removes the warnings.
> >
> > fritjof
> >
> >
> > Index: buf.c
> > ===================================================================
> > RCS file: /cvs/src/usr.bin/rcs/buf.c,v
> > retrieving revision 1.22
> > diff -u -p -r1.22 buf.c
> > --- buf.c       6 Jul 2011 15:36:52 -0000       1.22
> > +++ buf.c       6 May 2014 20:56:55 -0000
> > @@ -98,7 +98,7 @@ buf_load(const char *path)
> >         if (fstat(fd, &st) == -1)
> >                 goto out;
> >  
> > -       if (st.st_size > SIZE_MAX) {
> > +       if (st.st_size > (off_t)SIZE_MAX) {
> >                 errno = EFBIG;
> >                 goto out;
> >         }
> 
> This would break on 64 bits archs: there, SIZE_MAX casted to an off_t
> is -1 (implementation-defined behavior iirc).
> 
> Also it breaks platforms with 32 bits off_t (this does not affect
> OpenBSD).
> 
> I think the code is perfectly fine as is, but a possible clarification
> that does not do assumptions about the site of off_t and size_t could
> be:
> 
> if ((uintmax_t)st.st_size > (uintmax_t)SIZE_MAX)
>       ...
> 
> > Index: diff.c
> > ===================================================================
> > RCS file: /cvs/src/usr.bin/rcs/diff.c,v
> > retrieving revision 1.34
> > diff -u -p -r1.34 diff.c
> > --- diff.c      16 May 2013 12:44:48 -0000      1.34
> > +++ diff.c      6 May 2014 20:57:07 -0000
> > @@ -432,13 +432,13 @@ prepare(int i, FILE *fd, off_t filesize,
> >  
> >         rewind(fd);
> >  
> > -       sz = (filesize <= SIZE_MAX ? filesize : SIZE_MAX) / 25;
> > +       sz = (filesize <= (off_t)SIZE_MAX ? filesize : (off_t)SIZE_MAX) / 
> > 25;
> >         if (sz < 100)
> >                 sz = 100;
> 
> Same problem here.
> 
> >         p = xcalloc(sz + 3, sizeof(*p));
> >         for (j = 0; (h = readhash(fd, flags));) {
> > -               if (j == sz) {
> > +               if ((size_t)j == sz) {
> >                         sz = sz * 3 / 2;
> >                         p = xrealloc(p, sz + 3, sizeof(*p));
> >                 }
> 
> Declaring j as a size_t would save a cast.
> 
> > Index: diff3.c
> > ===================================================================
> > RCS file: /cvs/src/usr.bin/rcs/diff3.c,v
> > retrieving revision 1.33
> > diff -u -p -r1.33 diff3.c
> > --- diff3.c     4 Mar 2012 04:05:15 -0000       1.33
> > +++ diff3.c     6 May 2014 20:57:18 -0000
> > @@ -908,7 +908,7 @@ edscript(int n)
> >                 (void)fseek(fp[2], (long)de[n].new.from, SEEK_SET);
> >                 for (k = de[n].new.to-de[n].new.from; k > 0; k-= j) {
> >                         j = k > BUFSIZ ? BUFSIZ : k;
> > -                       if (fread(block, 1, j, fp[2]) != j)
> > +                       if ((int)fread(block, 1, j, fp[2]) != j)
> >                                 return (-1);
> >                         block[j] = '\0';
> >                         diff_output("%s", block);
> 
> Same here.
> 
> > Index: buf.c
> > ===================================================================
> > RCS file: /cvs/src/usr.bin/rcs/buf.c,v
> > retrieving revision 1.22
> > diff -u -p -r1.22 buf.c
> > --- buf.c       6 Jul 2011 15:36:52 -0000       1.22
> > +++ buf.c       6 May 2014 20:57:30 -0000
> > @@ -98,7 +98,7 @@ buf_load(const char *path)
> >         if (fstat(fd, &st) == -1)
> >                 goto out;
> >  
> > -       if (st.st_size > SIZE_MAX) {
> > +       if (st.st_size > (off_t)SIZE_MAX) {
> >                 errno = EFBIG;
> >                 goto out;
> >         }
> >
> 
> Duplicated hunk?
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Hi,

thank you for your feedback. I'll add it.

fritjof

Reply via email to