All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH for-4.6] xenstored: log tdb message via xenstored's logging mechanisms
@ 2014-12-15 13:18 Ian Campbell
  2015-01-08 12:33 ` Ian Campbell
  0 siblings, 1 reply; 7+ messages in thread
From: Ian Campbell @ 2014-12-15 13:18 UTC (permalink / raw)
  To: ian.jackson; +Cc: Philipp Hahn, Ian Campbell, xen-devel

TDB provides us with a callback for this purpose. Use it in both
xenstored and xs_tdb_dump.

While at it make the existing log() macro tollerate memory failures.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/xenstore/xenstored_core.c |   39 +++++++++++++++++++++++++++++++++------
 tools/xenstore/xs_tdb_dump.c    |   12 +++++++++++-
 2 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 4eaff57..3fd9a20 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -89,9 +89,14 @@ static void check_store(void);
 #define log(...)							\
 	do {								\
 		char *s = talloc_asprintf(NULL, __VA_ARGS__);		\
-		trace("%s\n", s);					\
-		syslog(LOG_ERR, "%s",  s);				\
-		talloc_free(s);						\
+		if (s) {						\
+			trace("%s\n", s);				\
+			syslog(LOG_ERR, "%s",  s);			\
+			talloc_free(s);					\
+		} else {						\
+			trace("talloc failure during logging\n");	\
+			syslog(LOG_ERR, "talloc failure during logging\n"); \
+		}							\
 	} while (0)
 
 
@@ -1479,13 +1484,35 @@ static void manual_node(const char *name, const char *child)
 	talloc_free(node);
 }
 
+static void tdb_logger(TDB_CONTEXT *tdb, int level, const char * fmt, ...)
+{
+	va_list ap;
+	char *s;
+
+	va_start(ap, fmt);
+	s = talloc_vasprintf(NULL, fmt, ap);
+	va_end(ap);
+
+	if (s) {
+		trace("TDB: %s\n", s);
+		syslog(LOG_ERR, "TDB: %s",  s);
+		if (verbose)
+			xprintf("TDB: %s", s);
+		talloc_free(s);
+	} else {
+		trace("talloc failure during logging\n");
+		syslog(LOG_ERR, "talloc failure during logging\n");
+	}
+}
+
 static void setup_structure(void)
 {
 	char *tdbname;
 	tdbname = talloc_strdup(talloc_autofree_context(), xs_daemon_tdb());
 
 	if (!(tdb_flags & TDB_INTERNAL))
-		tdb_ctx = tdb_open(tdbname, 0, tdb_flags, O_RDWR, 0);
+		tdb_ctx = tdb_open_ex(tdbname, 0, tdb_flags, O_RDWR, 0,
+				      &tdb_logger, NULL);
 
 	if (tdb_ctx) {
 		/* XXX When we make xenstored able to restart, this will have
@@ -1516,8 +1543,8 @@ static void setup_structure(void)
 		talloc_free(tlocal);
 	}
 	else {
-		tdb_ctx = tdb_open(tdbname, 7919, tdb_flags, O_RDWR|O_CREAT,
-				   0640);
+		tdb_ctx = tdb_open_ex(tdbname, 7919, tdb_flags, O_RDWR|O_CREAT,
+				      0640, &tdb_logger, NULL);
 		if (!tdb_ctx)
 			barf_perror("Could not create tdb file %s", tdbname);
 
diff --git a/tools/xenstore/xs_tdb_dump.c b/tools/xenstore/xs_tdb_dump.c
index b91cdef..7a034c0 100644
--- a/tools/xenstore/xs_tdb_dump.c
+++ b/tools/xenstore/xs_tdb_dump.c
@@ -33,6 +33,15 @@ static char perm_to_char(enum xs_perm_type perm)
 		'?';
 }
 
+static void tdb_logger(TDB_CONTEXT *tdb, int level, const char * fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	vfprintf(stderr, fmt, ap);
+	va_end(ap);
+}
+
 int main(int argc, char *argv[])
 {
 	TDB_DATA key;
@@ -41,7 +50,8 @@ int main(int argc, char *argv[])
 	if (argc != 2)
 		barf("Usage: xs_tdb_dump <tdbfile>");
 
-	tdb = tdb_open(talloc_strdup(NULL, argv[1]), 0, 0, O_RDONLY, 0);
+	tdb = tdb_open_ex(talloc_strdup(NULL, argv[1]), 0, 0, O_RDONLY, 0,
+			  tdb_logger, NULL);
 	if (!tdb)
 		barf_perror("Could not open %s", argv[1]);
 
-- 
1.7.10.4

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

* Re: [PATCH for-4.6] xenstored: log tdb message via xenstored's logging mechanisms
  2014-12-15 13:18 [PATCH for-4.6] xenstored: log tdb message via xenstored's logging mechanisms Ian Campbell
@ 2015-01-08 12:33 ` Ian Campbell
  2015-01-08 12:57   ` Wei Liu
  0 siblings, 1 reply; 7+ messages in thread
From: Ian Campbell @ 2015-01-08 12:33 UTC (permalink / raw)
  To: ian.jackson; +Cc: Philipp Hahn, Wei Liu, xen-devel

ping.

On Mon, 2014-12-15 at 13:18 +0000, Ian Campbell wrote:
> TDB provides us with a callback for this purpose. Use it in both
> xenstored and xs_tdb_dump.
> 
> While at it make the existing log() macro tollerate memory failures.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  tools/xenstore/xenstored_core.c |   39 +++++++++++++++++++++++++++++++++------
>  tools/xenstore/xs_tdb_dump.c    |   12 +++++++++++-
>  2 files changed, 44 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> index 4eaff57..3fd9a20 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -89,9 +89,14 @@ static void check_store(void);
>  #define log(...)							\
>  	do {								\
>  		char *s = talloc_asprintf(NULL, __VA_ARGS__);		\
> -		trace("%s\n", s);					\
> -		syslog(LOG_ERR, "%s",  s);				\
> -		talloc_free(s);						\
> +		if (s) {						\
> +			trace("%s\n", s);				\
> +			syslog(LOG_ERR, "%s",  s);			\
> +			talloc_free(s);					\
> +		} else {						\
> +			trace("talloc failure during logging\n");	\
> +			syslog(LOG_ERR, "talloc failure during logging\n"); \
> +		}							\
>  	} while (0)
>  
> 
> @@ -1479,13 +1484,35 @@ static void manual_node(const char *name, const char *child)
>  	talloc_free(node);
>  }
>  
> +static void tdb_logger(TDB_CONTEXT *tdb, int level, const char * fmt, ...)
> +{
> +	va_list ap;
> +	char *s;
> +
> +	va_start(ap, fmt);
> +	s = talloc_vasprintf(NULL, fmt, ap);
> +	va_end(ap);
> +
> +	if (s) {
> +		trace("TDB: %s\n", s);
> +		syslog(LOG_ERR, "TDB: %s",  s);
> +		if (verbose)
> +			xprintf("TDB: %s", s);
> +		talloc_free(s);
> +	} else {
> +		trace("talloc failure during logging\n");
> +		syslog(LOG_ERR, "talloc failure during logging\n");
> +	}
> +}
> +
>  static void setup_structure(void)
>  {
>  	char *tdbname;
>  	tdbname = talloc_strdup(talloc_autofree_context(), xs_daemon_tdb());
>  
>  	if (!(tdb_flags & TDB_INTERNAL))
> -		tdb_ctx = tdb_open(tdbname, 0, tdb_flags, O_RDWR, 0);
> +		tdb_ctx = tdb_open_ex(tdbname, 0, tdb_flags, O_RDWR, 0,
> +				      &tdb_logger, NULL);
>  
>  	if (tdb_ctx) {
>  		/* XXX When we make xenstored able to restart, this will have
> @@ -1516,8 +1543,8 @@ static void setup_structure(void)
>  		talloc_free(tlocal);
>  	}
>  	else {
> -		tdb_ctx = tdb_open(tdbname, 7919, tdb_flags, O_RDWR|O_CREAT,
> -				   0640);
> +		tdb_ctx = tdb_open_ex(tdbname, 7919, tdb_flags, O_RDWR|O_CREAT,
> +				      0640, &tdb_logger, NULL);
>  		if (!tdb_ctx)
>  			barf_perror("Could not create tdb file %s", tdbname);
>  
> diff --git a/tools/xenstore/xs_tdb_dump.c b/tools/xenstore/xs_tdb_dump.c
> index b91cdef..7a034c0 100644
> --- a/tools/xenstore/xs_tdb_dump.c
> +++ b/tools/xenstore/xs_tdb_dump.c
> @@ -33,6 +33,15 @@ static char perm_to_char(enum xs_perm_type perm)
>  		'?';
>  }
>  
> +static void tdb_logger(TDB_CONTEXT *tdb, int level, const char * fmt, ...)
> +{
> +	va_list ap;
> +
> +	va_start(ap, fmt);
> +	vfprintf(stderr, fmt, ap);
> +	va_end(ap);
> +}
> +
>  int main(int argc, char *argv[])
>  {
>  	TDB_DATA key;
> @@ -41,7 +50,8 @@ int main(int argc, char *argv[])
>  	if (argc != 2)
>  		barf("Usage: xs_tdb_dump <tdbfile>");
>  
> -	tdb = tdb_open(talloc_strdup(NULL, argv[1]), 0, 0, O_RDONLY, 0);
> +	tdb = tdb_open_ex(talloc_strdup(NULL, argv[1]), 0, 0, O_RDONLY, 0,
> +			  tdb_logger, NULL);
>  	if (!tdb)
>  		barf_perror("Could not open %s", argv[1]);
>  

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

* Re: [PATCH for-4.6] xenstored: log tdb message via xenstored's logging mechanisms
  2015-01-08 12:33 ` Ian Campbell
@ 2015-01-08 12:57   ` Wei Liu
  2015-01-08 13:04     ` Ian Campbell
  0 siblings, 1 reply; 7+ messages in thread
From: Wei Liu @ 2015-01-08 12:57 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Wei Liu, Philipp Hahn, ian.jackson, xen-devel

On Thu, Jan 08, 2015 at 12:33:29PM +0000, Ian Campbell wrote:
> ping.
> 
> On Mon, 2014-12-15 at 13:18 +0000, Ian Campbell wrote:
> > TDB provides us with a callback for this purpose. Use it in both
> > xenstored and xs_tdb_dump.
> > 
> > While at it make the existing log() macro tollerate memory failures.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> >  tools/xenstore/xenstored_core.c |   39 +++++++++++++++++++++++++++++++++------
> >  tools/xenstore/xs_tdb_dump.c    |   12 +++++++++++-
> >  2 files changed, 44 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> > index 4eaff57..3fd9a20 100644
> > --- a/tools/xenstore/xenstored_core.c
> > +++ b/tools/xenstore/xenstored_core.c
> > @@ -89,9 +89,14 @@ static void check_store(void);
> >  #define log(...)							\
> >  	do {								\
> >  		char *s = talloc_asprintf(NULL, __VA_ARGS__);		\
> > -		trace("%s\n", s);					\
> > -		syslog(LOG_ERR, "%s",  s);				\
> > -		talloc_free(s);						\
> > +		if (s) {						\
> > +			trace("%s\n", s);				\
> > +			syslog(LOG_ERR, "%s",  s);			\
> > +			talloc_free(s);					\
> > +		} else {						\
> > +			trace("talloc failure during logging\n");	\
> > +			syslog(LOG_ERR, "talloc failure during logging\n"); \
> > +		}							\

talloc_free can tolerate NULL pointer.

> >  	} while (0)
> >  
[...]
> > +		tdb_ctx = tdb_open_ex(tdbname, 0, tdb_flags, O_RDWR, 0,
> > +				      &tdb_logger, NULL);
[...]
> > +		tdb_ctx = tdb_open_ex(tdbname, 7919, tdb_flags, O_RDWR|O_CREAT,
> > +				      0640, &tdb_logger, NULL);
[...]
> > +	tdb = tdb_open_ex(talloc_strdup(NULL, argv[1]), 0, 0, O_RDONLY, 0,
> > +			  tdb_logger, NULL);

Should be &tdb_logger?

Wei.

> >  	if (!tdb)
> >  		barf_perror("Could not open %s", argv[1]);
> >  
> 

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

* Re: [PATCH for-4.6] xenstored: log tdb message via xenstored's logging mechanisms
  2015-01-08 12:57   ` Wei Liu
@ 2015-01-08 13:04     ` Ian Campbell
  2015-01-08 13:06       ` Wei Liu
  2015-01-08 13:10       ` Ian Campbell
  0 siblings, 2 replies; 7+ messages in thread
From: Ian Campbell @ 2015-01-08 13:04 UTC (permalink / raw)
  To: Wei Liu; +Cc: Philipp Hahn, ian.jackson, xen-devel

On Thu, 2015-01-08 at 12:57 +0000, Wei Liu wrote:
> On Thu, Jan 08, 2015 at 12:33:29PM +0000, Ian Campbell wrote:
> > ping.
> > 
> > On Mon, 2014-12-15 at 13:18 +0000, Ian Campbell wrote:
> > > TDB provides us with a callback for this purpose. Use it in both
> > > xenstored and xs_tdb_dump.
> > > 
> > > While at it make the existing log() macro tollerate memory failures.
> > > 
> > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > > ---
> > >  tools/xenstore/xenstored_core.c |   39 +++++++++++++++++++++++++++++++++------
> > >  tools/xenstore/xs_tdb_dump.c    |   12 +++++++++++-
> > >  2 files changed, 44 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> > > index 4eaff57..3fd9a20 100644
> > > --- a/tools/xenstore/xenstored_core.c
> > > +++ b/tools/xenstore/xenstored_core.c
> > > @@ -89,9 +89,14 @@ static void check_store(void);
> > >  #define log(...)							\
> > >  	do {								\
> > >  		char *s = talloc_asprintf(NULL, __VA_ARGS__);		\
> > > -		trace("%s\n", s);					\
> > > -		syslog(LOG_ERR, "%s",  s);				\
> > > -		talloc_free(s);						\
> > > +		if (s) {						\
> > > +			trace("%s\n", s);				\
> > > +			syslog(LOG_ERR, "%s",  s);			\
> > > +			talloc_free(s);					\
> > > +		} else {						\
> > > +			trace("talloc failure during logging\n");	\
> > > +			syslog(LOG_ERR, "talloc failure during logging\n"); \
> > > +		}							\
> 
> talloc_free can tolerate NULL pointer.

Good point, I'll fix. I think I'll also change the message to more
clearly indicate an allocation failure, since that is what has actually
happened.

> > >  	} while (0)
> > >  
> [...]
> > > +		tdb_ctx = tdb_open_ex(tdbname, 0, tdb_flags, O_RDWR, 0,
> > > +				      &tdb_logger, NULL);
> [...]
> > > +		tdb_ctx = tdb_open_ex(tdbname, 7919, tdb_flags, O_RDWR|O_CREAT,
> > > +				      0640, &tdb_logger, NULL);
> [...]
> > > +	tdb = tdb_open_ex(talloc_strdup(NULL, argv[1]), 0, 0, O_RDONLY, 0,
> > > +			  tdb_logger, NULL);
> 
> Should be &tdb_logger?

tdb_logger is a function, so it's not strictly required, but it is good
form (and a hint to the reader) plus inconsistent with the others, so
I'll change.

Ian.

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

* Re: [PATCH for-4.6] xenstored: log tdb message via xenstored's logging mechanisms
  2015-01-08 13:04     ` Ian Campbell
@ 2015-01-08 13:06       ` Wei Liu
  2015-01-08 13:10       ` Ian Campbell
  1 sibling, 0 replies; 7+ messages in thread
From: Wei Liu @ 2015-01-08 13:06 UTC (permalink / raw)
  To: Ian Campbell; +Cc: ian.jackson, Philipp Hahn, Wei Liu, xen-devel

On Thu, Jan 08, 2015 at 01:04:26PM +0000, Ian Campbell wrote:
> On Thu, 2015-01-08 at 12:57 +0000, Wei Liu wrote:
> > On Thu, Jan 08, 2015 at 12:33:29PM +0000, Ian Campbell wrote:
> > > ping.
> > > 
> > > On Mon, 2014-12-15 at 13:18 +0000, Ian Campbell wrote:
> > > > TDB provides us with a callback for this purpose. Use it in both
> > > > xenstored and xs_tdb_dump.
> > > > 
> > > > While at it make the existing log() macro tollerate memory failures.
> > > > 
> > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > > > ---
> > > >  tools/xenstore/xenstored_core.c |   39 +++++++++++++++++++++++++++++++++------
> > > >  tools/xenstore/xs_tdb_dump.c    |   12 +++++++++++-
> > > >  2 files changed, 44 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> > > > index 4eaff57..3fd9a20 100644
> > > > --- a/tools/xenstore/xenstored_core.c
> > > > +++ b/tools/xenstore/xenstored_core.c
> > > > @@ -89,9 +89,14 @@ static void check_store(void);
> > > >  #define log(...)							\
> > > >  	do {								\
> > > >  		char *s = talloc_asprintf(NULL, __VA_ARGS__);		\
> > > > -		trace("%s\n", s);					\
> > > > -		syslog(LOG_ERR, "%s",  s);				\
> > > > -		talloc_free(s);						\
> > > > +		if (s) {						\
> > > > +			trace("%s\n", s);				\
> > > > +			syslog(LOG_ERR, "%s",  s);			\
> > > > +			talloc_free(s);					\
> > > > +		} else {						\
> > > > +			trace("talloc failure during logging\n");	\
> > > > +			syslog(LOG_ERR, "talloc failure during logging\n"); \
> > > > +		}							\
> > 
> > talloc_free can tolerate NULL pointer.
> 
> Good point, I'll fix. I think I'll also change the message to more
> clearly indicate an allocation failure, since that is what has actually
> happened.
> 
> > > >  	} while (0)
> > > >  
> > [...]
> > > > +		tdb_ctx = tdb_open_ex(tdbname, 0, tdb_flags, O_RDWR, 0,
> > > > +				      &tdb_logger, NULL);
> > [...]
> > > > +		tdb_ctx = tdb_open_ex(tdbname, 7919, tdb_flags, O_RDWR|O_CREAT,
> > > > +				      0640, &tdb_logger, NULL);
> > [...]
> > > > +	tdb = tdb_open_ex(talloc_strdup(NULL, argv[1]), 0, 0, O_RDONLY, 0,
> > > > +			  tdb_logger, NULL);
> > 
> > Should be &tdb_logger?
> 
> tdb_logger is a function, so it's not strictly required, but it is good
> form (and a hint to the reader) plus inconsistent with the others, so
> I'll change.
> 

Yeah. I was more annoyed by the different styles in used. :-)

Wei.

> Ian.
> 

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

* Re: [PATCH for-4.6] xenstored: log tdb message via xenstored's logging mechanisms
  2015-01-08 13:04     ` Ian Campbell
  2015-01-08 13:06       ` Wei Liu
@ 2015-01-08 13:10       ` Ian Campbell
  2015-01-08 13:22         ` Wei Liu
  1 sibling, 1 reply; 7+ messages in thread
From: Ian Campbell @ 2015-01-08 13:10 UTC (permalink / raw)
  To: Wei Liu; +Cc: xen-devel, ian.jackson, Philipp Hahn

On Thu, 2015-01-08 at 13:04 +0000, Ian Campbell wrote:
> On Thu, 2015-01-08 at 12:57 +0000, Wei Liu wrote:
> > On Thu, Jan 08, 2015 at 12:33:29PM +0000, Ian Campbell wrote:
> > > ping.
> > > 
> > > On Mon, 2014-12-15 at 13:18 +0000, Ian Campbell wrote:
> > > > TDB provides us with a callback for this purpose. Use it in both
> > > > xenstored and xs_tdb_dump.
> > > > 
> > > > While at it make the existing log() macro tollerate memory failures.
> > > > 
> > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > > > ---
> > > >  tools/xenstore/xenstored_core.c |   39 +++++++++++++++++++++++++++++++++------
> > > >  tools/xenstore/xs_tdb_dump.c    |   12 +++++++++++-
> > > >  2 files changed, 44 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> > > > index 4eaff57..3fd9a20 100644
> > > > --- a/tools/xenstore/xenstored_core.c
> > > > +++ b/tools/xenstore/xenstored_core.c
> > > > @@ -89,9 +89,14 @@ static void check_store(void);
> > > >  #define log(...)							\
> > > >  	do {								\
> > > >  		char *s = talloc_asprintf(NULL, __VA_ARGS__);		\
> > > > -		trace("%s\n", s);					\
> > > > -		syslog(LOG_ERR, "%s",  s);				\
> > > > -		talloc_free(s);						\
> > > > +		if (s) {						\
> > > > +			trace("%s\n", s);				\
> > > > +			syslog(LOG_ERR, "%s",  s);			\
> > > > +			talloc_free(s);					\
> > > > +		} else {						\
> > > > +			trace("talloc failure during logging\n");	\
> > > > +			syslog(LOG_ERR, "talloc failure during logging\n"); \
> > > > +		}							\
> > 
> > talloc_free can tolerate NULL pointer.
> 
> Good point, I'll fix.

Hrm, it tolerates but it does return failure (-1) instead of success in
that case.

Literally nowhere in our code base actually checks the return value from
talloc_free, but nonetheless given that I think it would prefer to keep
the talloc_free where it is and therefore only pass a valid pointer to
it.

Ian.

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

* Re: [PATCH for-4.6] xenstored: log tdb message via xenstored's logging mechanisms
  2015-01-08 13:10       ` Ian Campbell
@ 2015-01-08 13:22         ` Wei Liu
  0 siblings, 0 replies; 7+ messages in thread
From: Wei Liu @ 2015-01-08 13:22 UTC (permalink / raw)
  To: Ian Campbell; +Cc: ian.jackson, xen-devel, Wei Liu, Philipp Hahn

On Thu, Jan 08, 2015 at 01:10:38PM +0000, Ian Campbell wrote:
> On Thu, 2015-01-08 at 13:04 +0000, Ian Campbell wrote:
> > On Thu, 2015-01-08 at 12:57 +0000, Wei Liu wrote:
> > > On Thu, Jan 08, 2015 at 12:33:29PM +0000, Ian Campbell wrote:
> > > > ping.
> > > > 
> > > > On Mon, 2014-12-15 at 13:18 +0000, Ian Campbell wrote:
> > > > > TDB provides us with a callback for this purpose. Use it in both
> > > > > xenstored and xs_tdb_dump.
> > > > > 
> > > > > While at it make the existing log() macro tollerate memory failures.
> > > > > 
> > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > > > > ---
> > > > >  tools/xenstore/xenstored_core.c |   39 +++++++++++++++++++++++++++++++++------
> > > > >  tools/xenstore/xs_tdb_dump.c    |   12 +++++++++++-
> > > > >  2 files changed, 44 insertions(+), 7 deletions(-)
> > > > > 
> > > > > diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> > > > > index 4eaff57..3fd9a20 100644
> > > > > --- a/tools/xenstore/xenstored_core.c
> > > > > +++ b/tools/xenstore/xenstored_core.c
> > > > > @@ -89,9 +89,14 @@ static void check_store(void);
> > > > >  #define log(...)							\
> > > > >  	do {								\
> > > > >  		char *s = talloc_asprintf(NULL, __VA_ARGS__);		\
> > > > > -		trace("%s\n", s);					\
> > > > > -		syslog(LOG_ERR, "%s",  s);				\
> > > > > -		talloc_free(s);						\
> > > > > +		if (s) {						\
> > > > > +			trace("%s\n", s);				\
> > > > > +			syslog(LOG_ERR, "%s",  s);			\
> > > > > +			talloc_free(s);					\
> > > > > +		} else {						\
> > > > > +			trace("talloc failure during logging\n");	\
> > > > > +			syslog(LOG_ERR, "talloc failure during logging\n"); \
> > > > > +		}							\
> > > 
> > > talloc_free can tolerate NULL pointer.
> > 
> > Good point, I'll fix.
> 
> Hrm, it tolerates but it does return failure (-1) instead of success in
> that case.
> 
> Literally nowhere in our code base actually checks the return value from
> talloc_free, but nonetheless given that I think it would prefer to keep
> the talloc_free where it is and therefore only pass a valid pointer to
> it.
> 

Fair enough.

Wei.

> Ian.

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

end of thread, other threads:[~2015-01-08 13:22 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-15 13:18 [PATCH for-4.6] xenstored: log tdb message via xenstored's logging mechanisms Ian Campbell
2015-01-08 12:33 ` Ian Campbell
2015-01-08 12:57   ` Wei Liu
2015-01-08 13:04     ` Ian Campbell
2015-01-08 13:06       ` Wei Liu
2015-01-08 13:10       ` Ian Campbell
2015-01-08 13:22         ` Wei Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.