mailing list of musl libc
 help / color / Atom feed
* [musl] [PATCH] implement recallocarray(3)
@ 2020-08-01 14:46 Ariadne Conill
  2020-08-01 15:52 ` Markus Wichmann
  0 siblings, 1 reply; 5+ messages in thread
From: Ariadne Conill @ 2020-08-01 14:46 UTC (permalink / raw)
  To: musl; +Cc: Ariadne Conill

This OpenBSD extension is similar to reallocarray(3), but
zero-initializes the new memory area.

This extension is placed in _BSD_SOURCE, like
reallocarray(3).

This patch depends on reallocarray(3) already being
present, and will not compile without it.
---
 include/stdlib.h           |  1 +
 src/malloc/recallocarray.c | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)
 create mode 100644 src/malloc/recallocarray.c

diff --git a/include/stdlib.h b/include/stdlib.h
index b54a051f..a0412ad4 100644
--- a/include/stdlib.h
+++ b/include/stdlib.h
@@ -146,6 +146,7 @@ int clearenv(void);
 #define WCOREDUMP(s) ((s) & 0x80)
 #define WIFCONTINUED(s) ((s) == 0xffff)
 void *reallocarray (void *, size_t, size_t);
+void *recallocarray (void *, size_t, size_t, size_t);
 #endif
 
 #ifdef _GNU_SOURCE
diff --git a/src/malloc/recallocarray.c b/src/malloc/recallocarray.c
new file mode 100644
index 00000000..c1370edc
--- /dev/null
+++ b/src/malloc/recallocarray.c
@@ -0,0 +1,33 @@
+#define _BSD_SOURCE
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+
+void *recallocarray(void *ptr, size_t om, size_t m, size_t n)
+{
+	void *newptr;
+	size_t old_size, new_size;
+
+	if (n && m > -1 / n) {
+		errno = ENOMEM;
+		return 0;
+	}
+	new_size = m * n;
+
+	if (n && om > -1 / n) {
+		errno = EINVAL;
+		return 0;
+	}
+	old_size = om * n;
+
+	if (new_size <= old_size) {
+		memset((char *) ptr + new_size, 0, old_size - new_size);
+	}
+
+	newptr = reallocarray(ptr, m, n);
+	if (new_size > old_size) {
+		memset((char *) ptr + old_size, 0, new_size - old_size);
+	}
+
+	return newptr;
+}
-- 
2.28.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [musl] [PATCH] implement recallocarray(3)
  2020-08-01 14:46 [musl] [PATCH] implement recallocarray(3) Ariadne Conill
@ 2020-08-01 15:52 ` Markus Wichmann
  2020-08-01 16:23   ` Ariadne Conill
  0 siblings, 1 reply; 5+ messages in thread
From: Markus Wichmann @ 2020-08-01 15:52 UTC (permalink / raw)
  To: musl

On Sat, Aug 01, 2020 at 08:46:58AM -0600, Ariadne Conill wrote:
> +void *recallocarray(void *ptr, size_t om, size_t m, size_t n)
> +{
> +	void *newptr;
> +	size_t old_size, new_size;
> +
> +	if (n && m > -1 / n) {
> +		errno = ENOMEM;
> +		return 0;
> +	}
> +	new_size = m * n;
> +
> +	if (n && om > -1 / n) {
> +		errno = EINVAL;
> +		return 0;
> +	}
> +	old_size = om * n;
> +
> +	if (new_size <= old_size) {
> +		memset((char *) ptr + new_size, 0, old_size - new_size);
> +	}
> +
> +	newptr = reallocarray(ptr, m, n);
> +	if (new_size > old_size) {
> +		memset((char *) ptr + old_size, 0, new_size - old_size);
> +	}
> +
> +	return newptr;
> +}

Is there a reason for the call to reallocarray? The multiplication m * n
has already been tested for overflow and executed at that point. Might
as well just call realloc() there, right?

JM2C,
Markus

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [musl] [PATCH] implement recallocarray(3)
  2020-08-01 15:52 ` Markus Wichmann
@ 2020-08-01 16:23   ` Ariadne Conill
  2020-08-01 17:46     ` Rich Felker
  0 siblings, 1 reply; 5+ messages in thread
From: Ariadne Conill @ 2020-08-01 16:23 UTC (permalink / raw)
  To: musl; +Cc: Markus Wichmann

On Saturday, 1 August 2020 09:52:28 MDT Markus Wichmann wrote:
> On Sat, Aug 01, 2020 at 08:46:58AM -0600, Ariadne Conill wrote:
> > +void *recallocarray(void *ptr, size_t om, size_t m, size_t n)
> > +{
> > +	void *newptr;
> > +	size_t old_size, new_size;
> > +
> > +	if (n && m > -1 / n) {
> > +		errno = ENOMEM;
> > +		return 0;
> > +	}
> > +	new_size = m * n;
> > +
> > +	if (n && om > -1 / n) {
> > +		errno = EINVAL;
> > +		return 0;
> > +	}
> > +	old_size = om * n;
> > +
> > +	if (new_size <= old_size) {
> > +		memset((char *) ptr + new_size, 0, old_size - new_size);
> > +	}
> > +
> > +	newptr = reallocarray(ptr, m, n);
> > +	if (new_size > old_size) {
> > +		memset((char *) ptr + old_size, 0, new_size - old_size);
> > +	}
> > +
> > +	return newptr;
> > +}
> 
> Is there a reason for the call to reallocarray? The multiplication m * n
> has already been tested for overflow and executed at that point. Might
> as well just call realloc() there, right?

Good catch.  I decided to use reallocarray() simply for clarity, but I can 
change it to a realloc() call.

Ariadne



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [musl] [PATCH] implement recallocarray(3)
  2020-08-01 16:23   ` Ariadne Conill
@ 2020-08-01 17:46     ` Rich Felker
  2020-08-01 17:51       ` Ariadne Conill
  0 siblings, 1 reply; 5+ messages in thread
From: Rich Felker @ 2020-08-01 17:46 UTC (permalink / raw)
  To: musl

On Sat, Aug 01, 2020 at 10:23:35AM -0600, Ariadne Conill wrote:
> On Saturday, 1 August 2020 09:52:28 MDT Markus Wichmann wrote:
> > On Sat, Aug 01, 2020 at 08:46:58AM -0600, Ariadne Conill wrote:
> > > +void *recallocarray(void *ptr, size_t om, size_t m, size_t n)
> > > +{
> > > +	void *newptr;
> > > +	size_t old_size, new_size;
> > > +
> > > +	if (n && m > -1 / n) {
> > > +		errno = ENOMEM;
> > > +		return 0;
> > > +	}
> > > +	new_size = m * n;
> > > +
> > > +	if (n && om > -1 / n) {
> > > +		errno = EINVAL;
> > > +		return 0;
> > > +	}
> > > +	old_size = om * n;
> > > +
> > > +	if (new_size <= old_size) {
> > > +		memset((char *) ptr + new_size, 0, old_size - new_size);
> > > +	}
> > > +
> > > +	newptr = reallocarray(ptr, m, n);
> > > +	if (new_size > old_size) {
> > > +		memset((char *) ptr + old_size, 0, new_size - old_size);
> > > +	}
> > > +
> > > +	return newptr;
> > > +}
> > 
> > Is there a reason for the call to reallocarray? The multiplication m * n
> > has already been tested for overflow and executed at that point. Might
> > as well just call realloc() there, right?
> 
> Good catch.  I decided to use reallocarray() simply for clarity, but I can 
> change it to a realloc() call.

I'm also confused why the old size has to be checked at all. There's
inherently a contract that the old size be correct for the existing
allocation; if it's not, the wrong number of members will be zeroed.
Checking whether om*n overflows does not change this and does not
catch the more general case where om is wrong.

Is the EINVAL behavior from OpenBSD, and if so, do they have a
rationale for it?

Rich

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [musl] [PATCH] implement recallocarray(3)
  2020-08-01 17:46     ` Rich Felker
@ 2020-08-01 17:51       ` Ariadne Conill
  0 siblings, 0 replies; 5+ messages in thread
From: Ariadne Conill @ 2020-08-01 17:51 UTC (permalink / raw)
  To: musl

Hello,

On Saturday, 1 August 2020 11:46:41 MDT Rich Felker wrote:
> On Sat, Aug 01, 2020 at 10:23:35AM -0600, Ariadne Conill wrote:
> > On Saturday, 1 August 2020 09:52:28 MDT Markus Wichmann wrote:
> > > On Sat, Aug 01, 2020 at 08:46:58AM -0600, Ariadne Conill wrote:
> > > > +void *recallocarray(void *ptr, size_t om, size_t m, size_t n)
> > > > +{
> > > > +	void *newptr;
> > > > +	size_t old_size, new_size;
> > > > +
> > > > +	if (n && m > -1 / n) {
> > > > +		errno = ENOMEM;
> > > > +		return 0;
> > > > +	}
> > > > +	new_size = m * n;
> > > > +
> > > > +	if (n && om > -1 / n) {
> > > > +		errno = EINVAL;
> > > > +		return 0;
> > > > +	}
> > > > +	old_size = om * n;
> > > > +
> > > > +	if (new_size <= old_size) {
> > > > +		memset((char *) ptr + new_size, 0, old_size - new_size);
> > > > +	}
> > > > +
> > > > +	newptr = reallocarray(ptr, m, n);
> > > > +	if (new_size > old_size) {
> > > > +		memset((char *) ptr + old_size, 0, new_size - old_size);
> > > > +	}
> > > > +
> > > > +	return newptr;
> > > > +}
> > > 
> > > Is there a reason for the call to reallocarray? The multiplication m * n
> > > has already been tested for overflow and executed at that point. Might
> > > as well just call realloc() there, right?
> > 
> > Good catch.  I decided to use reallocarray() simply for clarity, but I can
> > change it to a realloc() call.
> 
> I'm also confused why the old size has to be checked at all. There's
> inherently a contract that the old size be correct for the existing
> allocation; if it's not, the wrong number of members will be zeroed.
> Checking whether om*n overflows does not change this and does not
> catch the more general case where om is wrong.
> 
> Is the EINVAL behavior from OpenBSD, and if so, do they have a
> rationale for it?

Yes, EINVAL is mentioned in the man page.

    If ptr is not NULL and multiplying oldnmemb and size results in
    integer overflow recallocarray() returns NULL and sets errno to
    EINVAL.

They do not mention any rationale for returning EINVAL here, but they do clear 
any memory before reclaiming it on a shrink, so that may be related.  We could 
remove the check, but I added the check to be consistent with OpenBSD.

Ariadne



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-01 14:46 [musl] [PATCH] implement recallocarray(3) Ariadne Conill
2020-08-01 15:52 ` Markus Wichmann
2020-08-01 16:23   ` Ariadne Conill
2020-08-01 17:46     ` Rich Felker
2020-08-01 17:51       ` Ariadne Conill

mailing list of musl libc

Archives are clonable: git clone --mirror http://inbox.vuxu.org/musl

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.musl


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git