Hi. Thank you for helpful comments. I committed to the repository without #if..#else..#endif block.
http://cvs.savannah.gnu.org/viewcvs/global/libdb/mpool.c?cvsroot=global&r1=1.8&r2=1.9 On Wed, 27 Sep 2017 22:34:02 +0100 Punit Agrawal <[email protected]> wrote: > Hi Iwamoto-san, > > A couple of comments on the patch inline. > > Hideki IWAMOTO <[email protected]> writes: > > > Index: configure.ac > > =================================================================== > > RCS file: /sources/global/global/configure.ac,v > > retrieving revision 1.195 > > diff -u -p -r1.195 configure.ac > > --- configure.ac 15 May 2017 06:15:00 -0000 1.195 > > +++ configure.ac 27 Sep 2017 13:34:53 -0000 > > @@ -115,6 +115,8 @@ AC_CHECK_SIZEOF(char) > > if test ${ac_cv_sizeof_char} != 1; then > > AC_MSG_ERROR([Char size isn't 8 bit.]) > > fi > > +AC_CHECK_SIZEOF(long) > > +AC_CHECK_SIZEOF(off_t) > > I think this hunk isn't needed. More below... > > > > > dnl Checks for library functions. > > AC_FUNC_ALLOCA > > Index: libdb/mpool.c > > =================================================================== > > RCS file: /sources/global/global/libdb/mpool.c,v > > retrieving revision 1.8 > > diff -u -p -r1.8 mpool.c > > --- libdb/mpool.c 13 Oct 2012 07:01:21 -0000 1.8 > > +++ libdb/mpool.c 27 Sep 2017 13:34:53 -0000 > > @@ -229,7 +229,11 @@ mpool_get(mp, pgno, flags) > > #ifdef STATISTICS > > ++mp->pageread; > > #endif > > +#if SIZEOF_OFF_T > SIZEOF_LONG > > + off = (off_t)mp->pagesize * (off_t)pgno; > > There is no need to cast both the operands. Casting any one of the > operands is sufficient to promote the other. > > Also, it would be helpful to have a comment explaining the reason for > the cast. > > > +#else > > off = mp->pagesize * pgno; > > +#endif > > I think you can drop the #if checks and always cast unconditionally. It > is redundant if the sizeof(long) == sizeof(off_t) but it get's rid of > the #if..#else..#endif block. > > It also removes the need for the configure.ac hunk above. > > > #ifdef HAVE_PREAD > > if ((nr = pread(mp->fd, bp->page, mp->pagesize, off)) != mp->pagesize) { > > if (nr >= 0) > > @@ -425,7 +429,11 @@ mpool_write(mp, bp) > > if (mp->pgout) > > (mp->pgout)(mp->pgcookie, bp->pgno, bp->page); > > > > +#if SIZEOF_OFF_T > SIZEOF_LONG > > + off = (off_t)mp->pagesize * (off_t)bp->pgno; > > +#else > > off = mp->pagesize * bp->pgno; > > +#endif > > Same comment as above. > > Thanks, > Punit > > > #ifdef HAVE_PWRITE > > if (pwrite(mp->fd, bp->page, mp->pagesize, off) != mp->pagesize) > > return (RET_ERROR); > > > > _______________________________________________ > > Bug-global mailing list > > [email protected] > > https://lists.gnu.org/mailman/listinfo/bug-global -- 岩本 秀樹 <[email protected]> _______________________________________________ Bug-global mailing list [email protected] https://lists.gnu.org/mailman/listinfo/bug-global
