From ccc24a8e508131a2226474642a038baaa2cbcc8c Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Fri, 20 Jul 2018 13:42:13 -0700 Subject: fixes #605 NNI_ALLOC_STRUCT/NNI_ALLOC_STRUCTS should zero memory --- src/core/defs.h | 4 ++-- src/core/message.c | 10 +++++----- src/core/msgqueue.c | 4 ++-- src/core/platform.h | 13 +++++++++---- src/platform/posix/posix_alloc.c | 6 ++++++ src/platform/windows/win_thread.c | 8 +++++++- src/supplemental/http/http_msg.c | 36 ++++++++++++++---------------------- src/supplemental/tls/mbedtls/tls.c | 10 ++++------ 8 files changed, 49 insertions(+), 42 deletions(-) diff --git a/src/core/defs.h b/src/core/defs.h index a0cca368..3a3f23ff 100644 --- a/src/core/defs.h +++ b/src/core/defs.h @@ -82,9 +82,9 @@ typedef struct { #define NNI_SECOND (1000) // Structure allocation conveniences. -#define NNI_ALLOC_STRUCT(s) nni_alloc(sizeof(*s)) +#define NNI_ALLOC_STRUCT(s) nni_zalloc(sizeof(*s)) #define NNI_FREE_STRUCT(s) nni_free((s), sizeof(*s)) -#define NNI_ALLOC_STRUCTS(s, n) nni_alloc(sizeof(*s) * n) +#define NNI_ALLOC_STRUCTS(s, n) nni_zalloc(sizeof(*s) * n) #define NNI_FREE_STRUCTS(s, n) nni_free(s, sizeof(*s) * n) #define NNI_PUT16(ptr, u) \ diff --git a/src/core/message.c b/src/core/message.c index ba3c0e84..f17240a4 100644 --- a/src/core/message.c +++ b/src/core/message.c @@ -136,7 +136,7 @@ nni_chunk_grow(nni_chunk *ch, size_t newsz, size_t headwanted) newsz = ch->ch_cap - headroom; } - if ((newbuf = nni_alloc(newsz + headwanted)) == NULL) { + if ((newbuf = nni_zalloc(newsz + headwanted)) == NULL) { return (NNG_ENOMEM); } // Copy all the data, but not header or trailer. @@ -152,7 +152,7 @@ nni_chunk_grow(nni_chunk *ch, size_t newsz, size_t headwanted) // the backing store. In this case, we just check against the // allocated capacity and grow, or don't grow. if ((newsz + headwanted) >= ch->ch_cap) { - if ((newbuf = nni_alloc(newsz + headwanted)) == NULL) { + if ((newbuf = nni_zalloc(newsz + headwanted)) == NULL) { return (NNG_ENOMEM); } nni_free(ch->ch_buf, ch->ch_cap); @@ -215,7 +215,7 @@ nni_chunk_trim(nni_chunk *ch, size_t len) static int nni_chunk_dup(nni_chunk *dst, const nni_chunk *src) { - if ((dst->ch_buf = nni_alloc(src->ch_cap)) == NULL) { + if ((dst->ch_buf = nni_zalloc(src->ch_cap)) == NULL) { return (NNG_ENOMEM); } dst->ch_cap = src->ch_cap; @@ -387,7 +387,7 @@ nni_msg_dup(nni_msg **dup, const nni_msg *src) } NNI_LIST_FOREACH (&src->m_options, mo) { - newmo = nni_alloc(sizeof(*newmo) + mo->mo_sz); + newmo = nni_zalloc(sizeof(*newmo) + mo->mo_sz); if (newmo == NULL) { nni_msg_free(m); return (NNG_ENOMEM); @@ -436,7 +436,7 @@ nni_msg_setopt(nni_msg *m, int opt, const void *val, size_t sz) break; } } - if ((newmo = nni_alloc(sizeof(*newmo) + sz)) == NULL) { + if ((newmo = nni_zalloc(sizeof(*newmo) + sz)) == NULL) { return (NNG_ENOMEM); } newmo->mo_val = ((char *) newmo + sizeof(*newmo)); diff --git a/src/core/msgqueue.c b/src/core/msgqueue.c index 62f57553..e58fe6f5 100644 --- a/src/core/msgqueue.c +++ b/src/core/msgqueue.c @@ -57,7 +57,7 @@ nni_msgq_init(nni_msgq **mqp, unsigned cap) if ((mq = NNI_ALLOC_STRUCT(mq)) == NULL) { return (NNG_ENOMEM); } - if ((mq->mq_msgs = nni_alloc(sizeof(nng_msg *) * alloc)) == NULL) { + if ((mq->mq_msgs = nni_zalloc(sizeof(nng_msg *) * alloc)) == NULL) { NNI_FREE_STRUCT(mq); return (NNG_ENOMEM); } @@ -496,7 +496,7 @@ nni_msgq_resize(nni_msgq *mq, int cap) alloc = cap + 2; if (alloc > mq->mq_alloc) { - newq = nni_alloc(sizeof(nni_msg *) * alloc); + newq = nni_zalloc(sizeof(nni_msg *) * alloc); if (newq == NULL) { return (NNG_ENOMEM); } diff --git a/src/core/platform.h b/src/core/platform.h index 2ae0fd0b..70f4e9ef 100644 --- a/src/core/platform.h +++ b/src/core/platform.h @@ -69,10 +69,15 @@ extern const char *nni_plat_strerror(int); // to return NULL if memory cannot be allocated. extern void *nni_alloc(size_t); -// nni_free frees memory allocated with nni_alloc. It takes a size because -// some allocators do not track size, or can operate more efficiently if -// the size is provided with the free call. Examples of this are slab -// allocators like this found in Solaris/illumos (see libumem or kmem). +// nni_zalloc is just like nni_alloc, but ensures that memory is +// initialized to zero. It is a separate function because some platforms +// can use a more efficient zero-based allocation. +extern void *nni_zalloc(size_t); + +// nni_free frees memory allocated with nni_alloc or nni_zalloc. It takes +// a size because some allocators do not track size, or can operate more +// efficiently if the size is provided with the free call. Examples of this +// are slab allocators like this found in Solaris/illumos (see libumem). // This routine does nothing if supplied with a NULL pointer and zero size. // Most implementations can just call free() here. extern void nni_free(void *, size_t); diff --git a/src/platform/posix/posix_alloc.c b/src/platform/posix/posix_alloc.c index 3321f49c..f4b0245c 100644 --- a/src/platform/posix/posix_alloc.c +++ b/src/platform/posix/posix_alloc.c @@ -16,6 +16,12 @@ // POSIX memory allocation. This is pretty much standard C. void * nni_alloc(size_t sz) +{ + return (malloc(sz)); +} + +void * +nni_zalloc(size_t sz) { return (calloc(1, sz)); } diff --git a/src/platform/windows/win_thread.c b/src/platform/windows/win_thread.c index 52327cc4..d6363207 100644 --- a/src/platform/windows/win_thread.c +++ b/src/platform/windows/win_thread.c @@ -19,7 +19,13 @@ void * nni_alloc(size_t sz) { - return (calloc(sz, 1)); + return (malloc(sz)); +} + +void * +nni_zalloc(size_t sz) +{ + return (calloc(1, sz)); } void diff --git a/src/supplemental/http/http_msg.c b/src/supplemental/http/http_msg.c index ff9764bf..d6ab862e 100644 --- a/src/supplemental/http/http_msg.c +++ b/src/supplemental/http/http_msg.c @@ -77,12 +77,8 @@ http_headers_reset(nni_list *hdrs) http_header *h; while ((h = nni_list_first(hdrs)) != NULL) { nni_list_remove(hdrs, h); - if (h->name != NULL) { - nni_strfree(h->name); - } - if (h->value != NULL) { - nni_free(h->value, strlen(h->value) + 1); - } + nni_strfree(h->name); + nni_strfree(h->value); NNI_FREE_STRUCT(h); } } @@ -181,13 +177,11 @@ http_set_header(nni_list *hdrs, const char *key, const char *val) http_header *h; NNI_LIST_FOREACH (hdrs, h) { if (nni_strcasecmp(key, h->name) == 0) { - char * news; - size_t len = strlen(val) + 1; - if ((news = nni_alloc(len)) == NULL) { + char *news; + if ((news = nni_strdup(val)) == NULL) { return (NNG_ENOMEM); } - snprintf(news, len, "%s", val); - nni_free(h->value, strlen(h->value) + 1); + nni_strfree(h->value); h->value = news; return (0); } @@ -200,12 +194,11 @@ http_set_header(nni_list *hdrs, const char *key, const char *val) NNI_FREE_STRUCT(h); return (NNG_ENOMEM); } - if ((h->value = nni_alloc(strlen(val) + 1)) == NULL) { + if ((h->value = nni_strdup(val)) == NULL) { nni_strfree(h->name); NNI_FREE_STRUCT(h); return (NNG_ENOMEM); } - strncpy(h->value, val, strlen(val) + 1); nni_list_append(hdrs, h); return (0); } @@ -228,13 +221,13 @@ http_add_header(nni_list *hdrs, const char *key, const char *val) http_header *h; NNI_LIST_FOREACH (hdrs, h) { if (nni_strcasecmp(key, h->name) == 0) { - char * news; - size_t len = strlen(h->value) + strlen(val) + 3; - if ((news = nni_alloc(len)) == NULL) { - return (NNG_ENOMEM); + char *news; + int rv; + rv = nni_asprintf(&news, "%s, %s", h->value, val); + if (rv != 0) { + return (rv); } - snprintf(news, len, "%s, %s", h->value, val); - nni_free(h->value, strlen(h->value) + 1); + nni_strfree(h->value); h->value = news; return (0); } @@ -247,12 +240,11 @@ http_add_header(nni_list *hdrs, const char *key, const char *val) NNI_FREE_STRUCT(h); return (NNG_ENOMEM); } - if ((h->value = nni_alloc(strlen(val) + 1)) == NULL) { + if ((h->value = nni_strdup(val)) == NULL) { nni_strfree(h->name); NNI_FREE_STRUCT(h); return (NNG_ENOMEM); } - strncpy(h->value, val, strlen(val) + 1); nni_list_append(hdrs, h); return (0); } @@ -310,7 +302,7 @@ static int http_entity_alloc_data(nni_http_entity *entity, size_t size) { void *newdata; - if ((newdata = nni_alloc(size)) == NULL) { + if ((newdata = nni_zalloc(size)) == NULL) { return (NNG_ENOMEM); } http_entity_set_data(entity, newdata, size); diff --git a/src/supplemental/tls/mbedtls/tls.c b/src/supplemental/tls/mbedtls/tls.c index 0f4f67cc..42333783 100644 --- a/src/supplemental/tls/mbedtls/tls.c +++ b/src/supplemental/tls/mbedtls/tls.c @@ -319,11 +319,11 @@ nni_tls_init(nni_tls **tpp, nng_tls_config *cfg, nni_tcp_conn *tcp) if ((tp = NNI_ALLOC_STRUCT(tp)) == NULL) { return (NNG_ENOMEM); } - if ((tp->recvbuf = nni_alloc(NNG_TLS_MAX_RECV_SIZE)) == NULL) { + if ((tp->recvbuf = nni_zalloc(NNG_TLS_MAX_RECV_SIZE)) == NULL) { NNI_FREE_STRUCT(tp); return (NNG_ENOMEM); } - if ((tp->sendbuf = nni_alloc(NNG_TLS_MAX_SEND_SIZE)) == NULL) { + if ((tp->sendbuf = nni_zalloc(NNG_TLS_MAX_SEND_SIZE)) == NULL) { nni_free(tp->sendbuf, NNG_TLS_MAX_RECV_SIZE); NNI_FREE_STRUCT(tp); return (NNG_ENOMEM); @@ -957,12 +957,11 @@ nng_tls_config_ca_file(nng_tls_config *cfg, const char *path) if ((rv = nni_file_get(path, &fdata, &fsize)) != 0) { return (rv); } - if ((pem = nni_alloc(fsize + 1)) == NULL) { + if ((pem = nni_zalloc(fsize + 1)) == NULL) { nni_free(fdata, fsize); return (NNG_ENOMEM); } memcpy(pem, fdata, fsize); - pem[fsize] = '\0'; nni_free(fdata, fsize); if (strstr(pem, "-----BEGIN X509 CRL-----") != NULL) { rv = nng_tls_config_ca_chain(cfg, pem, pem); @@ -992,12 +991,11 @@ nng_tls_config_cert_key_file( if ((rv = nni_file_get(path, &fdata, &fsize)) != 0) { return (rv); } - if ((pem = nni_alloc(fsize + 1)) == NULL) { + if ((pem = nni_zalloc(fsize + 1)) == NULL) { nni_free(fdata, fsize); return (NNG_ENOMEM); } memcpy(pem, fdata, fsize); - pem[fsize] = '\0'; nni_free(fdata, fsize); rv = nng_tls_config_own_cert(cfg, pem, pem, pass); nni_free(pem, fsize + 1); -- cgit v1.2.3-70-g09d2