Skip to contentSkip to conversation

introduce SIZE_MAX

RFC PATCH In review Xi Wang 2 participants 3 messages Apr 19, 2012 – Apr 21, 2012

Subsystems (4)
DRM drivers DRM misc Slab allocator Ceph client
lore
Share & tools
lore

Xi Wang

author
#0 lore raw

RFC PATCH 1/1 introduce SIZE_MAX

ULONG_MAX is often used to check for integer overflow when calculating allocation size. While ULONG_MAX happens to work on most systems, there is no guarantee that size_t' must be the same size as long'.

This patch introduces SIZE_MAX, the maximum value of `size_t', to improve portability and readability for allocation size validation.

Signed-off-by Xi Wang <[email protected]>

---
This patch uses ceph and drm for demonstration purpose. I will resend and split the patch if the new macro is found useful.

---
 fs/ceph/snap.c             |    2 +-
 include/drm/drm_mem_util.h |    4 ++--
 include/linux/kernel.h     |    1 +
 include/linux/slab.h       |    2 +-
 4 files changed, 5 insertions(+), 4 deletions(-)
fs/ceph/snap.c +1 -1
index f04c096..e5206fc 100644
@@ -331,7 +331,7 @@ static int build_snap_context(struct ceph_snap_realm *realm)
 
 	/* alloc new snap context */
 	err = -ENOMEM;
-	if (num > (ULONG_MAX - sizeof(*snapc)) / sizeof(u64))
+	if (num > (SIZE_MAX - sizeof(*snapc)) / sizeof(u64))
 		goto fail;
 	snapc = kzalloc(sizeof(*snapc) + num*sizeof(u64), GFP_NOFS);
 	if (!snapc)
include/drm/drm_mem_util.h +2 -2
index 6bd325f..19a2404 100644
@@ -31,7 +31,7 @@
 
 static __inline__ void *drm_calloc_large(size_t nmemb, size_t size)
 {
-	if (size != 0 && nmemb > ULONG_MAX / size)
+	if (size != 0 && nmemb > SIZE_MAX / size)
 		return NULL;
 
 	if (size * nmemb <= PAGE_SIZE)

@@ -44,7 +44,7 @@ static __inline__ void *drm_calloc_large(size_t nmemb, size_t size)
 /* Modeled after cairo's malloc_ab, it's like calloc but without the zeroing. */
 static __inline__ void *drm_malloc_ab(size_t nmemb, size_t size)
 {
-	if (size != 0 && nmemb > ULONG_MAX / size)
+	if (size != 0 && nmemb > SIZE_MAX / size)
 		return NULL;
 
 	if (size * nmemb <= PAGE_SIZE)
include/linux/kernel.h +1 -0
index 645231c..b795ee5 100644
@@ -35,6 +35,7 @@
 #define LLONG_MAX	((long long)(~0ULL>>1))
 #define LLONG_MIN	(-LLONG_MAX - 1)
 #define ULLONG_MAX	(~0ULL)
+#define SIZE_MAX	(~(size_t)0)
 
 #define STACK_MAGIC	0xdeadbeef
include/linux/slab.h +1 -1
index a595dce..67d5d94 100644
@@ -242,7 +242,7 @@ size_t ksize(const void *);
  */
 static inline void *kmalloc_array(size_t n, size_t size, gfp_t flags)
 {
-	if (size != 0 && n > ULONG_MAX / size)
+	if (size != 0 && n > SIZE_MAX / size)
 		return NULL;
 	return __kmalloc(n * size, flags);
 }
Signature · 1 line
1.7.5.4
maintainer
#1 lore raw
Quoted 10 lines · On Thu, 19 Apr 2012 00:27:51 -0400 Xi Wang <[email protected]> wrote:

ULONG_MAX is often used to check for integer overflow when calculating allocation size. While ULONG_MAX happens to work on most systems, there is no guarantee that size_t' must be the same size as long'.

This patch introduces SIZE_MAX, the maximum value of `size_t', to improve portability and readability for allocation size validation.

...

fs/ceph/snap.c static int build_snap_context(struct ceph_snap_realm *realm) · line 337

Quoted 8 lines
fs/ceph/snap.c +1 -1
@@ -331,7 +331,7 @@ static int build_snap_context(struct ceph_snap_realm *realm)
 
 	/* alloc new snap context */
 	err = -ENOMEM;
-	if (num > (ULONG_MAX - sizeof(*snapc)) / sizeof(u64))
+	if (num > (SIZE_MAX - sizeof(*snapc)) / sizeof(u64))
 		goto fail;
 	snapc = kzalloc(sizeof(*snapc) + num*sizeof(u64), GFP_NOFS);
 	if (!snapc)

hm, yes, I suppose that's better - hardwiring the assumption that size_t has type unsigned long is pretty ugly.

Will we need something for ssize_t also?

author
#2 lore raw
Quoted 1 line · Andrew Morton

Will we need something for ssize_t also?

Did you mean something like SSIZE_MAX for replacing LONG_MAX? That's a good question since we have ssize_t.

I tried to grep LONG_MAX' and size'. The only place I have found so far is the following:

fs/xfs/xfs_aops.c:1247, in __xfs_get_blocks()

	if (mapping_size > LONG_MAX)
		mapping_size = LONG_MAX;

	bh_result->b_size = mapping_size;

Not sure if SSIZE_MAX works better here (`b_size' is size_t).

- xi