[PATCH 2/3] util: add xtalloc.[ch]

[Austin Clements]

Quoth david at tethera.net on Dec 16 at 11:24 pm:
> From: David Bremner 
> 
> These are intended to be simple wrappers to provide slightly better
> debugging information than what talloc currently provides natively.
> ---
>  notmuch-client.h|2 +-
>  util/Makefile.local |2 +-
>  util/xtalloc.c  |   15 +++
>  util/xtalloc.h  |   18 ++
>  4 files changed, 35 insertions(+), 2 deletions(-)
>  create mode 100644 util/xtalloc.c
>  create mode 100644 util/xtalloc.h
> 
> diff --git a/notmuch-client.h b/notmuch-client.h
> index d7b352e..60be030 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -58,7 +58,7 @@ typedef GMimeCipherContext notmuch_crypto_context_t;
>  #include 
>  #include 
>  
> -#include 
> +#include "xtalloc.h"
>  
>  #define unused(x) x __attribute__ ((unused))
>  
> diff --git a/util/Makefile.local b/util/Makefile.local
> index a11e35b..8a62c00 100644
> --- a/util/Makefile.local
> +++ b/util/Makefile.local
> @@ -4,7 +4,7 @@ dir := util
>  extra_cflags += -I$(srcdir)/$(dir)
>  
>  libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \
> -   $(dir)/string-util.c
> +   $(dir)/string-util.c $(dir)/xtalloc.c
>  
>  libutil_modules := $(libutil_c_srcs:.c=.o)
>  
> diff --git a/util/xtalloc.c b/util/xtalloc.c
> new file mode 100644
> index 000..22834bd
> --- /dev/null
> +++ b/util/xtalloc.c
> @@ -0,0 +1,15 @@
> +#include 
> +#include "xtalloc.h"
> +
> +char *
> +xtalloc_strndup_named_const (void *ctx, const char *str,
> +  size_t len, const char *name)
> +{
> +char *ptr = talloc_named_const (ctx, len + 1, name);
> +
> +if (ptr) {
> + memcpy (ptr, str, len);

This isn't safe.  If the string at ptr is actually shorter than len,
this may read past allocated memory and crash.

Maybe this should just call talloc_strndup and talloc_set_name_const?

> + *(ptr + len) = '\0';
> +}
> +return ptr;
> +}
> diff --git a/util/xtalloc.h b/util/xtalloc.h
> new file mode 100644
> index 000..3cc1179
> --- /dev/null
> +++ b/util/xtalloc.h
> @@ -0,0 +1,18 @@
> +#ifndef _XTALLOC_H
> +#define _XTALLOC_H
> +
> +#include 
> +
> +/* Like talloc_strndup, but take an extra parameter for the internal talloc
> + * name (for debugging) */
> +
> +char *
> +xtalloc_strndup_named_const (void *ctx, const char *str,
> +  size_t len, const char *name);

I agree with Tomi that these shouldn't be named with 'x'.  For this
one, it seems fine to simply drop the 'x', since the name is fully
descriptive of what it does.

> +
> +/* use the __location__ macro from talloc.h to name a string according to its
> + * source location */
> +
> +#define xtalloc_strndup(ctx, str, len) xtalloc_strndup_named_const (ctx, 
> str, len, __location__)

For this, what about talloc_strndup_debug?

> +
> +#endif

[PATCH 2/3] util: add xtalloc.[ch]

[Tomi Ollila]

On Mon, Dec 17 2012, david at tethera.net wrote:

> From: David Bremner 
>
> These are intended to be simple wrappers to provide slightly better
> debugging information than what talloc currently provides natively.
> ---

AFAIC it's been customary to have memory allocation functions starting
with 'x' to abort execution if memory allocation fails. See util/xutil.c
in notmuch source for reference. IMHO it would be confusing to use
the 'x' prefix for some other use.

Tomi


>  notmuch-client.h|2 +-
>  util/Makefile.local |2 +-
>  util/xtalloc.c  |   15 +++
>  util/xtalloc.h  |   18 ++
>  4 files changed, 35 insertions(+), 2 deletions(-)
>  create mode 100644 util/xtalloc.c
>  create mode 100644 util/xtalloc.h
>
> diff --git a/notmuch-client.h b/notmuch-client.h
> index d7b352e..60be030 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -58,7 +58,7 @@ typedef GMimeCipherContext notmuch_crypto_context_t;
>  #include 
>  #include 
>  
> -#include 
> +#include "xtalloc.h"
>  
>  #define unused(x) x __attribute__ ((unused))
>  
> diff --git a/util/Makefile.local b/util/Makefile.local
> index a11e35b..8a62c00 100644
> --- a/util/Makefile.local
> +++ b/util/Makefile.local
> @@ -4,7 +4,7 @@ dir := util
>  extra_cflags += -I$(srcdir)/$(dir)
>  
>  libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \
> -   $(dir)/string-util.c
> +   $(dir)/string-util.c $(dir)/xtalloc.c
>  
>  libutil_modules := $(libutil_c_srcs:.c=.o)
>  
> diff --git a/util/xtalloc.c b/util/xtalloc.c
> new file mode 100644
> index 000..22834bd
> --- /dev/null
> +++ b/util/xtalloc.c
> @@ -0,0 +1,15 @@
> +#include 
> +#include "xtalloc.h"
> +
> +char *
> +xtalloc_strndup_named_const (void *ctx, const char *str,
> +  size_t len, const char *name)
> +{
> +char *ptr = talloc_named_const (ctx, len + 1, name);
> +
> +if (ptr) {
> + memcpy (ptr, str, len);
> + *(ptr + len) = '\0';
> +}
> +return ptr;
> +}
> diff --git a/util/xtalloc.h b/util/xtalloc.h
> new file mode 100644
> index 000..3cc1179
> --- /dev/null
> +++ b/util/xtalloc.h
> @@ -0,0 +1,18 @@
> +#ifndef _XTALLOC_H
> +#define _XTALLOC_H
> +
> +#include 
> +
> +/* Like talloc_strndup, but take an extra parameter for the internal talloc
> + * name (for debugging) */
> +
> +char *
> +xtalloc_strndup_named_const (void *ctx, const char *str,
> +  size_t len, const char *name);
> +
> +/* use the __location__ macro from talloc.h to name a string according to its
> + * source location */
> +
> +#define xtalloc_strndup(ctx, str, len) xtalloc_strndup_named_const (ctx, 
> str, len, __location__)
> +
> +#endif
> -- 
> 1.7.10.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 2/3] util: add xtalloc.[ch]

[Tomi Ollila]

On Mon, Dec 17 2012, da...@tethera.net wrote:

 From: David Bremner brem...@debian.org

 These are intended to be simple wrappers to provide slightly better
 debugging information than what talloc currently provides natively.
 ---

AFAIC it's been customary to have memory allocation functions starting
with 'x' to abort execution if memory allocation fails. See util/xutil.c
in notmuch source for reference. IMHO it would be confusing to use
the 'x' prefix for some other use.

Tomi


  notmuch-client.h|2 +-
  util/Makefile.local |2 +-
  util/xtalloc.c  |   15 +++
  util/xtalloc.h  |   18 ++
  4 files changed, 35 insertions(+), 2 deletions(-)
  create mode 100644 util/xtalloc.c
  create mode 100644 util/xtalloc.h

 diff --git a/notmuch-client.h b/notmuch-client.h
 index d7b352e..60be030 100644
 --- a/notmuch-client.h
 +++ b/notmuch-client.h
 @@ -58,7 +58,7 @@ typedef GMimeCipherContext notmuch_crypto_context_t;
  #include errno.h
  #include signal.h
  
 -#include talloc.h
 +#include xtalloc.h
  
  #define unused(x) x __attribute__ ((unused))
  
 diff --git a/util/Makefile.local b/util/Makefile.local
 index a11e35b..8a62c00 100644
 --- a/util/Makefile.local
 +++ b/util/Makefile.local
 @@ -4,7 +4,7 @@ dir := util
  extra_cflags += -I$(srcdir)/$(dir)
  
  libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \
 -   $(dir)/string-util.c
 +   $(dir)/string-util.c $(dir)/xtalloc.c
  
  libutil_modules := $(libutil_c_srcs:.c=.o)
  
 diff --git a/util/xtalloc.c b/util/xtalloc.c
 new file mode 100644
 index 000..22834bd
 --- /dev/null
 +++ b/util/xtalloc.c
 @@ -0,0 +1,15 @@
 +#include string.h
 +#include xtalloc.h
 +
 +char *
 +xtalloc_strndup_named_const (void *ctx, const char *str,
 +  size_t len, const char *name)
 +{
 +char *ptr = talloc_named_const (ctx, len + 1, name);
 +
 +if (ptr) {
 + memcpy (ptr, str, len);
 + *(ptr + len) = '\0';
 +}
 +return ptr;
 +}
 diff --git a/util/xtalloc.h b/util/xtalloc.h
 new file mode 100644
 index 000..3cc1179
 --- /dev/null
 +++ b/util/xtalloc.h
 @@ -0,0 +1,18 @@
 +#ifndef _XTALLOC_H
 +#define _XTALLOC_H
 +
 +#include talloc.h
 +
 +/* Like talloc_strndup, but take an extra parameter for the internal talloc
 + * name (for debugging) */
 +
 +char *
 +xtalloc_strndup_named_const (void *ctx, const char *str,
 +  size_t len, const char *name);
 +
 +/* use the __location__ macro from talloc.h to name a string according to its
 + * source location */
 +
 +#define xtalloc_strndup(ctx, str, len) xtalloc_strndup_named_const (ctx, 
 str, len, __location__)
 +
 +#endif
 -- 
 1.7.10.4

 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 2/3] util: add xtalloc.[ch]

[Austin Clements]

Quoth da...@tethera.net on Dec 16 at 11:24 pm:
 From: David Bremner brem...@debian.org
 
 These are intended to be simple wrappers to provide slightly better
 debugging information than what talloc currently provides natively.
 ---
  notmuch-client.h|2 +-
  util/Makefile.local |2 +-
  util/xtalloc.c  |   15 +++
  util/xtalloc.h  |   18 ++
  4 files changed, 35 insertions(+), 2 deletions(-)
  create mode 100644 util/xtalloc.c
  create mode 100644 util/xtalloc.h
 
 diff --git a/notmuch-client.h b/notmuch-client.h
 index d7b352e..60be030 100644
 --- a/notmuch-client.h
 +++ b/notmuch-client.h
 @@ -58,7 +58,7 @@ typedef GMimeCipherContext notmuch_crypto_context_t;
  #include errno.h
  #include signal.h
  
 -#include talloc.h
 +#include xtalloc.h
  
  #define unused(x) x __attribute__ ((unused))
  
 diff --git a/util/Makefile.local b/util/Makefile.local
 index a11e35b..8a62c00 100644
 --- a/util/Makefile.local
 +++ b/util/Makefile.local
 @@ -4,7 +4,7 @@ dir := util
  extra_cflags += -I$(srcdir)/$(dir)
  
  libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \
 -   $(dir)/string-util.c
 +   $(dir)/string-util.c $(dir)/xtalloc.c
  
  libutil_modules := $(libutil_c_srcs:.c=.o)
  
 diff --git a/util/xtalloc.c b/util/xtalloc.c
 new file mode 100644
 index 000..22834bd
 --- /dev/null
 +++ b/util/xtalloc.c
 @@ -0,0 +1,15 @@
 +#include string.h
 +#include xtalloc.h
 +
 +char *
 +xtalloc_strndup_named_const (void *ctx, const char *str,
 +  size_t len, const char *name)
 +{
 +char *ptr = talloc_named_const (ctx, len + 1, name);
 +
 +if (ptr) {
 + memcpy (ptr, str, len);

This isn't safe.  If the string at ptr is actually shorter than len,
this may read past allocated memory and crash.

Maybe this should just call talloc_strndup and talloc_set_name_const?

 + *(ptr + len) = '\0';
 +}
 +return ptr;
 +}
 diff --git a/util/xtalloc.h b/util/xtalloc.h
 new file mode 100644
 index 000..3cc1179
 --- /dev/null
 +++ b/util/xtalloc.h
 @@ -0,0 +1,18 @@
 +#ifndef _XTALLOC_H
 +#define _XTALLOC_H
 +
 +#include talloc.h
 +
 +/* Like talloc_strndup, but take an extra parameter for the internal talloc
 + * name (for debugging) */
 +
 +char *
 +xtalloc_strndup_named_const (void *ctx, const char *str,
 +  size_t len, const char *name);

I agree with Tomi that these shouldn't be named with 'x'.  For this
one, it seems fine to simply drop the 'x', since the name is fully
descriptive of what it does.

 +
 +/* use the __location__ macro from talloc.h to name a string according to its
 + * source location */
 +
 +#define xtalloc_strndup(ctx, str, len) xtalloc_strndup_named_const (ctx, 
 str, len, __location__)

For this, what about talloc_strndup_debug?

 +
 +#endif
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 2/3] util: add xtalloc.[ch]

[da...@tethera.net]

From: David Bremner 

These are intended to be simple wrappers to provide slightly better
debugging information than what talloc currently provides natively.
---
 notmuch-client.h|2 +-
 util/Makefile.local |2 +-
 util/xtalloc.c  |   15 +++
 util/xtalloc.h  |   18 ++
 4 files changed, 35 insertions(+), 2 deletions(-)
 create mode 100644 util/xtalloc.c
 create mode 100644 util/xtalloc.h

diff --git a/notmuch-client.h b/notmuch-client.h
index d7b352e..60be030 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -58,7 +58,7 @@ typedef GMimeCipherContext notmuch_crypto_context_t;
 #include 
 #include 

-#include 
+#include "xtalloc.h"

 #define unused(x) x __attribute__ ((unused))

diff --git a/util/Makefile.local b/util/Makefile.local
index a11e35b..8a62c00 100644
--- a/util/Makefile.local
+++ b/util/Makefile.local
@@ -4,7 +4,7 @@ dir := util
 extra_cflags += -I$(srcdir)/$(dir)

 libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \
- $(dir)/string-util.c
+ $(dir)/string-util.c $(dir)/xtalloc.c

 libutil_modules := $(libutil_c_srcs:.c=.o)

diff --git a/util/xtalloc.c b/util/xtalloc.c
new file mode 100644
index 000..22834bd
--- /dev/null
+++ b/util/xtalloc.c
@@ -0,0 +1,15 @@
+#include 
+#include "xtalloc.h"
+
+char *
+xtalloc_strndup_named_const (void *ctx, const char *str,
+size_t len, const char *name)
+{
+char *ptr = talloc_named_const (ctx, len + 1, name);
+
+if (ptr) {
+   memcpy (ptr, str, len);
+   *(ptr + len) = '\0';
+}
+return ptr;
+}
diff --git a/util/xtalloc.h b/util/xtalloc.h
new file mode 100644
index 000..3cc1179
--- /dev/null
+++ b/util/xtalloc.h
@@ -0,0 +1,18 @@
+#ifndef _XTALLOC_H
+#define _XTALLOC_H
+
+#include 
+
+/* Like talloc_strndup, but take an extra parameter for the internal talloc
+ * name (for debugging) */
+
+char *
+xtalloc_strndup_named_const (void *ctx, const char *str,
+size_t len, const char *name);
+
+/* use the __location__ macro from talloc.h to name a string according to its
+ * source location */
+
+#define xtalloc_strndup(ctx, str, len) xtalloc_strndup_named_const (ctx, str, 
len, __location__)
+
+#endif
-- 
1.7.10.4

[PATCH 2/3] util: add xtalloc.[ch]

[david]

From: David Bremner brem...@debian.org

These are intended to be simple wrappers to provide slightly better
debugging information than what talloc currently provides natively.
---
 notmuch-client.h|2 +-
 util/Makefile.local |2 +-
 util/xtalloc.c  |   15 +++
 util/xtalloc.h  |   18 ++
 4 files changed, 35 insertions(+), 2 deletions(-)
 create mode 100644 util/xtalloc.c
 create mode 100644 util/xtalloc.h

diff --git a/notmuch-client.h b/notmuch-client.h
index d7b352e..60be030 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -58,7 +58,7 @@ typedef GMimeCipherContext notmuch_crypto_context_t;
 #include errno.h
 #include signal.h
 
-#include talloc.h
+#include xtalloc.h
 
 #define unused(x) x __attribute__ ((unused))
 
diff --git a/util/Makefile.local b/util/Makefile.local
index a11e35b..8a62c00 100644
--- a/util/Makefile.local
+++ b/util/Makefile.local
@@ -4,7 +4,7 @@ dir := util
 extra_cflags += -I$(srcdir)/$(dir)
 
 libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \
- $(dir)/string-util.c
+ $(dir)/string-util.c $(dir)/xtalloc.c
 
 libutil_modules := $(libutil_c_srcs:.c=.o)
 
diff --git a/util/xtalloc.c b/util/xtalloc.c
new file mode 100644
index 000..22834bd
--- /dev/null
+++ b/util/xtalloc.c
@@ -0,0 +1,15 @@
+#include string.h
+#include xtalloc.h
+
+char *
+xtalloc_strndup_named_const (void *ctx, const char *str,
+size_t len, const char *name)
+{
+char *ptr = talloc_named_const (ctx, len + 1, name);
+
+if (ptr) {
+   memcpy (ptr, str, len);
+   *(ptr + len) = '\0';
+}
+return ptr;
+}
diff --git a/util/xtalloc.h b/util/xtalloc.h
new file mode 100644
index 000..3cc1179
--- /dev/null
+++ b/util/xtalloc.h
@@ -0,0 +1,18 @@
+#ifndef _XTALLOC_H
+#define _XTALLOC_H
+
+#include talloc.h
+
+/* Like talloc_strndup, but take an extra parameter for the internal talloc
+ * name (for debugging) */
+
+char *
+xtalloc_strndup_named_const (void *ctx, const char *str,
+size_t len, const char *name);
+
+/* use the __location__ macro from talloc.h to name a string according to its
+ * source location */
+
+#define xtalloc_strndup(ctx, str, len) xtalloc_strndup_named_const (ctx, str, 
len, __location__)
+
+#endif
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch