On Tue, Mar 19, 2013 at 10:47 AM, Ted Unangst <[email protected]> wrote:
> Maybe we want, maybe we don't. Here's a check to prevent a counter
> from going negative, which usually means an accounting mistake
> somewhere. On the shortcomiing side, it only catches double
> decrements, not missing decrements, so maybe it's not worthwhile.
> Anyway, it was like five minutes to make, figured I'd show it.
>
> Index: lib/libkern/libkern.h
> ===================================================================
> RCS file: /cvs/src/sys/lib/libkern/libkern.h,v
> retrieving revision 1.29
> diff -u -p -r1.29 libkern.h
> --- lib/libkern/libkern.h 26 Apr 2012 01:22:31 -0000 1.29
> +++ lib/libkern/libkern.h 19 Mar 2013 15:10:41 -0000
> @@ -116,14 +116,11 @@ abs(int j)
>
> #ifndef DIAGNOSTIC
> #define KASSERT(e) ((void)0)
> +#define DECREMENT(x) x--
why not (x)--?
> #else
> -#ifdef __STDC__
> #define KASSERT(e) ((e) ? (void)0 :
> \
> __assert("diagnostic ", __FILE__, __LINE__, #e))
> -#else
> -#define KASSERT(e) ((e) ? (void)0 :
> \
> - __assert("diagnostic ", __FILE__, __LINE__, "e"))
> -#endif
> +#define DECREMENT(x) decrement_checked((x)--, __FILE__, __LINE__, #x)
why (x)-- instead of x-- as before?
> #endif
>
> #ifndef DEBUG
> @@ -141,6 +138,7 @@ abs(int j)
> /* Prototypes for non-quad routines. */
> void __assert(const char *, const char *, int, const char *)
> __attribute__ ((__noreturn__));
> +int64_t decrement_checked(int64_t, const char *, int, const char *);
why not intmax_t instead of int64_t?
> int bcmp(const void *, const void *, size_t);
> void bzero(void *, size_t);
> void explicit_bzero(void *, size_t);
> Index: kern/subr_prf.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/subr_prf.c,v
> retrieving revision 1.76
> diff -u -p -r1.76 subr_prf.c
> --- kern/subr_prf.c 3 Apr 2011 16:46:19 -0000 1.76
> +++ kern/subr_prf.c 19 Mar 2013 15:13:27 -0000
> @@ -161,6 +161,14 @@ __assert(const char *t, const char *f, i
> t, e, f, l);
> }
>
> +int64_t
> +decrement_checked(int64_t x, const char *f, int l, const char *e)
> +{
> + if (x <= 0)
> + __assert("decrement ", f, l, e);
> + return x;
> +}
> +
> /*
> * tablefull: warn that a system table is full
> */
> Index: kern/vfs_bio.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/vfs_bio.c,v
> retrieving revision 1.146
> diff -u -p -r1.146 vfs_bio.c
> --- kern/vfs_bio.c 17 Feb 2013 17:39:29 -0000 1.146
> +++ kern/vfs_bio.c 19 Mar 2013 15:12:12 -0000
> @@ -129,7 +129,7 @@ bremfree(struct buf *bp)
> bcstats.numcleanpages -= atop(bp->b_bufsize);
> } else {
> bcstats.numdirtypages -= atop(bp->b_bufsize);
> - bcstats.delwribufs--;
> + DECREMENT(bcstats.delwribufs);
> }
> TAILQ_REMOVE(dp, bp, b_freelist);
> }
> @@ -155,7 +155,7 @@ buf_put(struct buf *bp)
> #endif
>
> LIST_REMOVE(bp, b_list);
> - bcstats.numbufs--;
> + DECREMENT(bcstats.numbufs);
>
> if (buf_dealloc_mem(bp) != 0)
> return;
> @@ -1221,9 +1221,9 @@ biodone(struct buf *bp)
> if (bcstats.numbufs &&
> (!(ISSET(bp->b_flags, B_RAW) || ISSET(bp->b_flags, B_PHYS)))) {
> if (!ISSET(bp->b_flags, B_READ))
> - bcstats.pendingwrites--;
> + DECREMENT(bcstats.pendingwrites);
> else
> - bcstats.pendingreads--;
> + DECREMENT(bcstats.pendingreads);
> }
> if (ISSET(bp->b_flags, B_CALL)) { /* if necessary, call out */
> CLR(bp->b_flags, B_CALL); /* but note callout done */
>
