[PATCH] Minor fixes to generic do_div

[PATCH] Minor fixes to generic do_div

[Matthew Wilcox]

While tracking down a problem recently, I noticed the description of
do_div() is not quite right.  We also don't check that 'n' is 64-bit
when running on a 64-bit machine (this would have caused warnings to
appear on more configurations if we had).  And finally, the indentation
of the \ characters in the 64-bit case don't match those in the 32-bit case,
so change those to match.

Signed-off-by: Matthew Wilcox <matthew@wil.cx>

diff --git a/include/asm-generic/div64.h b/include/asm-generic/div64.h
index 8f4e319..0edfb96 100644
--- a/include/asm-generic/div64.h
+++ b/include/asm-generic/div64.h
@@ -6,10 +6,10 @@ #define _ASM_GENERIC_DIV64_H
  *
  * The semantics of do_div() are:
  *
- * uint32_t do_div(uint64_t *n, uint32_t base)
+ * uint32_t do_div(uint64_t n, uint32_t base)
  * {
- * 	uint32_t remainder = *n % base;
- * 	*n = *n / base;
+ * 	uint32_t remainder = n % base;
+ * 	n = n / base;
  * 	return remainder;
  * }
  *
@@ -22,12 +22,16 @@ #include <linux/compiler.h>
 
 #if BITS_PER_LONG == 64
 
-# define do_div(n,base) ({					\
-	uint32_t __base = (base);				\
-	uint32_t __rem;						\
-	__rem = ((uint64_t)(n)) % __base;			\
-	(n) = ((uint64_t)(n)) / __base;				\
-	__rem;							\
+/* The unnecessary pointer compare is there
+ * to check for type safety (n must be 64bit)
+ */
+# define do_div(n,base) ({				\
+	uint32_t __base = (base);			\
+	uint32_t __rem;					\
+	(void)(((typeof((n)) *)0) == ((uint64_t *)0));	\
+	__rem = ((uint64_t)(n)) % __base;		\
+	(n) = ((uint64_t)(n)) / __base;			\
+	__rem;						\
  })
 
 #elif BITS_PER_LONG == 32

Re: [PATCH] Minor fixes to generic do_div

[Andrew Morton]

On Thu, 19 Oct 2006 21:34:00 -0600
Matthew Wilcox <matthew@wil.cx> wrote:

> +/* The unnecessary pointer compare is there
> + * to check for type safety (n must be 64bit)
> + */
> +# define do_div(n,base) ({				\
> +	uint32_t __base = (base);			\
> +	uint32_t __rem;					\
> +	(void)(((typeof((n)) *)0) == ((uint64_t *)0));	\
> +	__rem = ((uint64_t)(n)) % __base;		\
> +	(n) = ((uint64_t)(n)) / __base;			\
> +	__rem;						\
>   })

Can we use typecheck(), from include/linux/kernel.h?

Re: [PATCH] Minor fixes to generic do_div

[Matthew Wilcox]

On Thu, Oct 19, 2006 at 09:59:54PM -0700, Andrew Morton wrote:
> Can we use typecheck(), from include/linux/kernel.h?

I don't know.

It's copied and pasted from down below, so possibly this was
intentionally not used.  or possibly the author didn't know about
typecheck().

If we do use it, we either have to include linux/kernel.h in
asm-generic/div64.h, which drags in a slew of includes of its own, or be
sure that all users already include kernel.h.  i bet they do.

My allmodconfig build is currently testing out the
remove-sched.h-from-asm-parisc-uaccess.h patch based on viro's x86-64
patch seen earlier today, so I won't be testing the second hypothesis
tonight.  Anyone want to try plugging in typecheck() and seeing if
anything breaks?  NB: you'll want to be sure your arch is using
asm-generic/div64, or add the typecheck() to your arch's
asm-foo/div64.h.

We should probably do that anyway, at least for i386.  And then someone
else would maybe wonder what the xtensa port is up to.

Re: [PATCH] Minor fixes to generic do_div

[Andrew Morton]

On Thu, 19 Oct 2006 23:36:38 -0600
Matthew Wilcox <matthew@wil.cx> wrote:

> On Thu, Oct 19, 2006 at 09:59:54PM -0700, Andrew Morton wrote:
> > Can we use typecheck(), from include/linux/kernel.h?
> 
> I don't know.
> 
> It's copied and pasted from down below, so possibly this was
> intentionally not used.  or possibly the author didn't know about
> typecheck().

or typecheck() was added afterwards.

One could create a typecheck.h.  Or a handy-macros.h whose mandate is
"macros which don't depend on any other headers".

Or just include kernel.h in div64.h.

Re: [PATCH] Minor fixes to generic do_div

[Paul Mackerras]

Matthew Wilcox writes:

>   * The semantics of do_div() are:
>   *
> - * uint32_t do_div(uint64_t *n, uint32_t base)
> + * uint32_t do_div(uint64_t n, uint32_t base)

... or would be, if n were passed by reference.  If you're going to
change the comment, I think you should say explicitly that n is
changed.

Paul.