Re: [Xen-devel] [PATCH for-4.5 1/3] python/xc: Fix multiple issues in pyflask_context_to_sid()
On Fri, 2014-11-28 at 11:37 +, Ian Campbell wrote: On Thu, 2014-11-27 at 12:34 +, Andrew Cooper wrote: The error handling from a failed memory allocation should return PyErr_SetFromErrno(xc_error_obj); rather than simply calling it and continuing to the memcpy() below, with the dest pointer being NULL. Furthermore, the context string is simply an input parameter to the hypercall, and is not mutated anywhere along the way. The error handling elsewhere in the function can be simplified by not duplicating it to start with. Signed-off-by: Andrew Cooper andrew.coop...@citrix.com Coverity-IDs: 1055305 1055721 CC: Ian Campbell ian.campb...@citrix.com CC: Ian Jackson ian.jack...@eu.citrix.com CC: Wei Liu wei.l...@citrix.com CC: Xen Coverity Team cover...@xen.org Acked-by: Ian Campbell ian.campb...@citrix.com This would have been far more obviously correct for 4.5 if you had stuck to fixing the issue in the first paragraph. Konrad, given http://article.gmane.org/gmane.comp.emulators.xen.devel/224881 does this have a release ack? Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.5 1/3] python/xc: Fix multiple issues in pyflask_context_to_sid()
On 09/12/14 14:27, Ian Campbell wrote: On Fri, 2014-11-28 at 11:37 +, Ian Campbell wrote: On Thu, 2014-11-27 at 12:34 +, Andrew Cooper wrote: The error handling from a failed memory allocation should return PyErr_SetFromErrno(xc_error_obj); rather than simply calling it and continuing to the memcpy() below, with the dest pointer being NULL. Furthermore, the context string is simply an input parameter to the hypercall, and is not mutated anywhere along the way. The error handling elsewhere in the function can be simplified by not duplicating it to start with. Signed-off-by: Andrew Cooper andrew.coop...@citrix.com Coverity-IDs: 1055305 1055721 CC: Ian Campbell ian.campb...@citrix.com CC: Ian Jackson ian.jack...@eu.citrix.com CC: Wei Liu wei.l...@citrix.com CC: Xen Coverity Team cover...@xen.org Acked-by: Ian Campbell ian.campb...@citrix.com This would have been far more obviously correct for 4.5 if you had stuck to fixing the issue in the first paragraph. Konrad, given http://article.gmane.org/gmane.comp.emulators.xen.devel/224881 does this have a release ack? Ian. I can resubmit with a clearer description if that would help clarity, but the code is correct for the fixes (not fantastically well) described. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.5 1/3] python/xc: Fix multiple issues in pyflask_context_to_sid()
On Tue, Dec 09, 2014 at 02:30:24PM +, Andrew Cooper wrote: On 09/12/14 14:27, Ian Campbell wrote: On Fri, 2014-11-28 at 11:37 +, Ian Campbell wrote: On Thu, 2014-11-27 at 12:34 +, Andrew Cooper wrote: The error handling from a failed memory allocation should return PyErr_SetFromErrno(xc_error_obj); rather than simply calling it and continuing to the memcpy() below, with the dest pointer being NULL. Furthermore, the context string is simply an input parameter to the hypercall, and is not mutated anywhere along the way. The error handling elsewhere in the function can be simplified by not duplicating it to start with. Signed-off-by: Andrew Cooper andrew.coop...@citrix.com Coverity-IDs: 1055305 1055721 CC: Ian Campbell ian.campb...@citrix.com CC: Ian Jackson ian.jack...@eu.citrix.com CC: Wei Liu wei.l...@citrix.com CC: Xen Coverity Team cover...@xen.org Acked-by: Ian Campbell ian.campb...@citrix.com This would have been far more obviously correct for 4.5 if you had stuck to fixing the issue in the first paragraph. Konrad, given http://article.gmane.org/gmane.comp.emulators.xen.devel/224881 does this have a release ack? Ian. I can resubmit with a clearer description if that would help clarity, but the code is correct for the fixes (not fantastically well) described. Please do - that is all I was waiting for. Thank you. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.5 1/3] python/xc: Fix multiple issues in pyflask_context_to_sid()
On Thu, 2014-11-27 at 12:34 +, Andrew Cooper wrote:
The error handling from a failed memory allocation should return
PyErr_SetFromErrno(xc_error_obj); rather than simply calling it and continuing
to the memcpy() below, with the dest pointer being NULL.
Furthermore, the context string is simply an input parameter to the hypercall,
and is not mutated anywhere along the way. The error handling elsewhere in
the function can be simplified by not duplicating it to start with.
Signed-off-by: Andrew Cooper andrew.coop...@citrix.com
Coverity-IDs: 1055305 1055721
CC: Ian Campbell ian.campb...@citrix.com
CC: Ian Jackson ian.jack...@eu.citrix.com
CC: Wei Liu wei.l...@citrix.com
CC: Xen Coverity Team cover...@xen.org
Acked-by: Ian Campbell ian.campb...@citrix.com
This would have been far more obviously correct for 4.5 if you had stuck
to fixing the issue in the first paragraph.
---
tools/python/xen/lowlevel/xc/xc.c | 21 +++--
1 file changed, 3 insertions(+), 18 deletions(-)
diff --git a/tools/python/xen/lowlevel/xc/xc.c
b/tools/python/xen/lowlevel/xc/xc.c
index d95d459..c70b388 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -2126,8 +2126,6 @@ static PyObject *pyflask_context_to_sid(PyObject *self,
PyObject *args,
{
xc_interface *xc_handle;
char *ctx;
-char *buf;
-uint32_t len;
uint32_t sid;
int ret;
@@ -2137,28 +2135,15 @@ static PyObject *pyflask_context_to_sid(PyObject
*self, PyObject *args,
ctx) )
return NULL;
-len = strlen(ctx);
-
-buf = malloc(len);
-if (!buf) {
-errno = -ENOMEM;
-PyErr_SetFromErrno(xc_error_obj);
-}
-
-memcpy(buf, ctx, len);
-
xc_handle = xc_interface_open(0,0,0);
if (!xc_handle) {
-free(buf);
return PyErr_SetFromErrno(xc_error_obj);
}
-
-ret = xc_flask_context_to_sid(xc_handle, buf, len, sid);
-
+
+ret = xc_flask_context_to_sid(xc_handle, ctx, strlen(ctx), sid);
+
xc_interface_close(xc_handle);
-free(buf);
-
if ( ret != 0 ) {
errno = -ret;
return PyErr_SetFromErrno(xc_error_obj);
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.5 1/3] python/xc: Fix multiple issues in pyflask_context_to_sid()
On Fri, 2014-11-28 at 11:47 +, Andrew Cooper wrote: On 28/11/14 11:37, Ian Campbell wrote: On Thu, 2014-11-27 at 12:34 +, Andrew Cooper wrote: The error handling from a failed memory allocation should return PyErr_SetFromErrno(xc_error_obj); rather than simply calling it and continuing to the memcpy() below, with the dest pointer being NULL. Furthermore, the context string is simply an input parameter to the hypercall, and is not mutated anywhere along the way. The error handling elsewhere in the function can be simplified by not duplicating it to start with. Signed-off-by: Andrew Cooper andrew.coop...@citrix.com Coverity-IDs: 1055305 1055721 CC: Ian Campbell ian.campb...@citrix.com CC: Ian Jackson ian.jack...@eu.citrix.com CC: Wei Liu wei.l...@citrix.com CC: Xen Coverity Team cover...@xen.org Acked-by: Ian Campbell ian.campb...@citrix.com This would have been far more obviously correct for 4.5 if you had stuck to fixing the issue in the first paragraph. Hmm - I appear to have missed a detail in the description. One of the CIDs is to do with putting a string in a new buffer without a NUL terminator, and passing it as a char* into xc_flask_context_to_sid; the issue being that anyone expecting things like strlen() to work will be in for a nasty shock. ISTR a discussion of this interface in Julien's device-tree passthrough thing a while back and some sort of conclusion that the hypervisor was supposed to use the len field and not rely on the NULL. I'm not sure that necessarily invalidates what you are saying, since even given that throwing a NULL on the end would be friendly to libxc consumers if nothing else. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH for-4.5 1/3] python/xc: Fix multiple issues in pyflask_context_to_sid()
The error handling from a failed memory allocation should return
PyErr_SetFromErrno(xc_error_obj); rather than simply calling it and continuing
to the memcpy() below, with the dest pointer being NULL.
Furthermore, the context string is simply an input parameter to the hypercall,
and is not mutated anywhere along the way. The error handling elsewhere in
the function can be simplified by not duplicating it to start with.
Signed-off-by: Andrew Cooper andrew.coop...@citrix.com
Coverity-IDs: 1055305 1055721
CC: Ian Campbell ian.campb...@citrix.com
CC: Ian Jackson ian.jack...@eu.citrix.com
CC: Wei Liu wei.l...@citrix.com
CC: Xen Coverity Team cover...@xen.org
---
tools/python/xen/lowlevel/xc/xc.c | 21 +++--
1 file changed, 3 insertions(+), 18 deletions(-)
diff --git a/tools/python/xen/lowlevel/xc/xc.c
b/tools/python/xen/lowlevel/xc/xc.c
index d95d459..c70b388 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -2126,8 +2126,6 @@ static PyObject *pyflask_context_to_sid(PyObject *self,
PyObject *args,
{
xc_interface *xc_handle;
char *ctx;
-char *buf;
-uint32_t len;
uint32_t sid;
int ret;
@@ -2137,28 +2135,15 @@ static PyObject *pyflask_context_to_sid(PyObject *self,
PyObject *args,
ctx) )
return NULL;
-len = strlen(ctx);
-
-buf = malloc(len);
-if (!buf) {
-errno = -ENOMEM;
-PyErr_SetFromErrno(xc_error_obj);
-}
-
-memcpy(buf, ctx, len);
-
xc_handle = xc_interface_open(0,0,0);
if (!xc_handle) {
-free(buf);
return PyErr_SetFromErrno(xc_error_obj);
}
-
-ret = xc_flask_context_to_sid(xc_handle, buf, len, sid);
-
+
+ret = xc_flask_context_to_sid(xc_handle, ctx, strlen(ctx), sid);
+
xc_interface_close(xc_handle);
-free(buf);
-
if ( ret != 0 ) {
errno = -ret;
return PyErr_SetFromErrno(xc_error_obj);
--
1.7.10.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
