All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ocaml/xenctrl: Make failwith_xc() thread safe
@ 2015-01-28 17:55 Andrew Cooper
  2015-01-30 14:19 ` Dave Scott
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Cooper @ 2015-01-28 17:55 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Ian Jackson, Dave Scott, Ian Campbell

The static error_str[] buffer is not thread-safe, and 1024 bytes is
unreasonably large.  Reduce to 256 bytes (which is still much larger than any
current use), and move it to being a stack variable.

Also, propagate the Noreturn attribute from caml_raise_with_string().

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Dave Scott <Dave.Scott@eu.citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 tools/ocaml/libs/xc/xenctrl_stubs.c |   15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
index 92d064f..6a95528 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -51,21 +51,22 @@
 	i1 = (uint32_t) Int64_val(Field(input, 0)); \
 	i2 = ((Field(input, 1) == Val_none) ? 0xffffffff : (uint32_t) Int64_val(Field(Field(input, 1), 0)));
 
-#define ERROR_STRLEN 1024
-void failwith_xc(xc_interface *xch)
+static void Noreturn failwith_xc(xc_interface *xch)
 {
-	static char error_str[ERROR_STRLEN];
+	char error_str[256];
 	if (xch) {
 		const xc_error *error = xc_get_last_error(xch);
 		if (error->code == XC_ERROR_NONE)
-                	snprintf(error_str, ERROR_STRLEN, "%d: %s", errno, strerror(errno));
+			snprintf(error_str, sizeof(error_str),
+				 "%d: %s", errno, strerror(errno));
 		else
-			snprintf(error_str, ERROR_STRLEN, "%d: %s: %s",
-				 error->code,
+			snprintf(error_str, sizeof(error_str),
+				 "%d: %s: %s", error->code,
 				 xc_error_code_to_desc(error->code),
 				 error->message);
 	} else {
-		snprintf(error_str, ERROR_STRLEN, "Unable to open XC interface");
+		snprintf(error_str, sizeof(error_str),
+			 "Unable to open XC interface");
 	}
 	caml_raise_with_string(*caml_named_value("xc.error"), error_str);
 }
-- 
1.7.10.4

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

* Re: [PATCH] ocaml/xenctrl: Make failwith_xc() thread safe
  2015-01-28 17:55 [PATCH] ocaml/xenctrl: Make failwith_xc() thread safe Andrew Cooper
@ 2015-01-30 14:19 ` Dave Scott
  2015-01-30 14:20   ` Wei Liu
  0 siblings, 1 reply; 5+ messages in thread
From: Dave Scott @ 2015-01-30 14:19 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Ian Jackson, Dave Scott, Wei Liu, Ian Campbell, Xen-devel


Looks ok to me.

Signed-off-by: David Scott <dave.scott@citrix.com>

> On 28 Jan 2015, at 17:55, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> 
> The static error_str[] buffer is not thread-safe, and 1024 bytes is
> unreasonably large.  Reduce to 256 bytes (which is still much larger than any
> current use), and move it to being a stack variable.
> 
> Also, propagate the Noreturn attribute from caml_raise_with_string().
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Dave Scott <Dave.Scott@eu.citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> ---
> tools/ocaml/libs/xc/xenctrl_stubs.c |   15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
> index 92d064f..6a95528 100644
> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c
> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
> @@ -51,21 +51,22 @@
> 	i1 = (uint32_t) Int64_val(Field(input, 0)); \
> 	i2 = ((Field(input, 1) == Val_none) ? 0xffffffff : (uint32_t) Int64_val(Field(Field(input, 1), 0)));
> 
> -#define ERROR_STRLEN 1024
> -void failwith_xc(xc_interface *xch)
> +static void Noreturn failwith_xc(xc_interface *xch)
> {
> -	static char error_str[ERROR_STRLEN];
> +	char error_str[256];
> 	if (xch) {
> 		const xc_error *error = xc_get_last_error(xch);
> 		if (error->code == XC_ERROR_NONE)
> -                	snprintf(error_str, ERROR_STRLEN, "%d: %s", errno, strerror(errno));
> +			snprintf(error_str, sizeof(error_str),
> +				 "%d: %s", errno, strerror(errno));
> 		else
> -			snprintf(error_str, ERROR_STRLEN, "%d: %s: %s",
> -				 error->code,
> +			snprintf(error_str, sizeof(error_str),
> +				 "%d: %s: %s", error->code,
> 				 xc_error_code_to_desc(error->code),
> 				 error->message);
> 	} else {
> -		snprintf(error_str, ERROR_STRLEN, "Unable to open XC interface");
> +		snprintf(error_str, sizeof(error_str),
> +			 "Unable to open XC interface");
> 	}
> 	caml_raise_with_string(*caml_named_value("xc.error"), error_str);
> }
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

* Re: [PATCH] ocaml/xenctrl: Make failwith_xc() thread safe
  2015-01-30 14:19 ` Dave Scott
@ 2015-01-30 14:20   ` Wei Liu
  2015-01-30 14:24     ` Dave Scott
  0 siblings, 1 reply; 5+ messages in thread
From: Wei Liu @ 2015-01-30 14:20 UTC (permalink / raw)
  To: Dave Scott; +Cc: Andrew Cooper, Ian Jackson, Wei Liu, Ian Campbell, Xen-devel

On Fri, Jan 30, 2015 at 02:19:53PM +0000, Dave Scott wrote:
> 
> Looks ok to me.
> 
> Signed-off-by: David Scott <dave.scott@citrix.com>
> 

I think this should be an Acked-by.

Wei.

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

* Re: [PATCH] ocaml/xenctrl: Make failwith_xc() thread safe
  2015-01-30 14:20   ` Wei Liu
@ 2015-01-30 14:24     ` Dave Scott
  2015-02-02 15:29       ` Ian Campbell
  0 siblings, 1 reply; 5+ messages in thread
From: Dave Scott @ 2015-01-30 14:24 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, Ian Jackson, Ian Campbell, Xen-devel


> On 30 Jan 2015, at 14:20, Wei Liu <wei.liu2@citrix.com> wrote:
> 
> On Fri, Jan 30, 2015 at 02:19:53PM +0000, Dave Scott wrote:
>> 
>> Looks ok to me.
>> 
>> Signed-off-by: David Scott <dave.scott@citrix.com>
>> 
> 
> I think this should be an Acked-by.

Sorry, you’re completely right. Muscle memory strikes again! :)

Acked-by: David Scott <dave.scott@citrix.com>

> 
> Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

* Re: [PATCH] ocaml/xenctrl: Make failwith_xc() thread safe
  2015-01-30 14:24     ` Dave Scott
@ 2015-02-02 15:29       ` Ian Campbell
  0 siblings, 0 replies; 5+ messages in thread
From: Ian Campbell @ 2015-02-02 15:29 UTC (permalink / raw)
  To: Dave Scott; +Cc: Andrew Cooper, Ian Jackson, Wei Liu, Xen-devel

On Fri, 2015-01-30 at 14:24 +0000, Dave Scott wrote:
> 
> > On 30 Jan 2015, at 14:20, Wei Liu <wei.liu2@citrix.com> wrote:
> > 
> > On Fri, Jan 30, 2015 at 02:19:53PM +0000, Dave Scott wrote:
> >> 
> >> Looks ok to me.
> >> 
> >> Signed-off-by: David Scott <dave.scott@citrix.com>
> >> 
> > 
> > I think this should be an Acked-by.
> 
> Sorry, you’re completely right. Muscle memory strikes again! :)

;-)

> Acked-by: David Scott <dave.scott@citrix.com>

Thanks, Applied






_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

end of thread, other threads:[~2015-02-02 15:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-28 17:55 [PATCH] ocaml/xenctrl: Make failwith_xc() thread safe Andrew Cooper
2015-01-30 14:19 ` Dave Scott
2015-01-30 14:20   ` Wei Liu
2015-01-30 14:24     ` Dave Scott
2015-02-02 15:29       ` Ian Campbell

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.