[libvirt] [PATCH libvirt-python 1/2] setPyVirTypedParameter: Copy full field name
In the setPyVirTypedParameter we try to produce virTypedParameter array from a pyhthon dictionary. However, when copying field name into item in returned array, we use strncpy() as the field name is fixed length array. To determine its size we use sizeof() but mistakenly dereference it resulting in sizeof(char) which equals to 1 byte. Moreover, there's no need for using sizeof() when we have a global macro to tell us the length of the field name: VIR_TYPED_PARAM_FIELD_LENGTH. And since array is allocated using VIR_ALLOC() we are sure the memory is initially filled with zeros. Hence, there's no need to terminate string we've just copied into field name with '\0' character. It's there for sure too as we copy up to field length - 1. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- libvirt-override.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libvirt-override.c b/libvirt-override.c index 83369fc..3765a43 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -197,8 +197,7 @@ setPyVirTypedParameter(PyObject *info, goto cleanup; } - strncpy(temp->field, keystr, sizeof(*temp->field) - 1); - temp->field[sizeof(*temp->field) - 1] = '\0'; + strncpy(temp->field, keystr, VIR_TYPED_PARAM_FIELD_LENGTH - 1); temp->type = params[i].type; VIR_FREE(keystr); -- 1.9.0
The @ret value is built in a loop. However, if in one iteration there's an error, we should free all the fields built so far. For instance, if there's an error and the previous item was type of VIR_TYPED_PARAM_STRING we definitely must free it. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- libvirt-override.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-override.c b/libvirt-override.c index 3765a43..7f746ed 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -259,7 +259,7 @@ setPyVirTypedParameter(PyObject *info, return ret; cleanup: - VIR_FREE(ret); + virTypedParamsFree(ret, size); return NULL; } -- 1.9.0
On Tue, Mar 18, 2014 at 09:26:09AM +0100, Michal Privoznik wrote:
The @ret value is built in a loop. However, if in one iteration there's an error, we should free all the fields built so far. For instance, if there's an error and the previous item was type of VIR_TYPED_PARAM_STRING we definitely must free it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- libvirt-override.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ACK, Martin
diff --git a/libvirt-override.c b/libvirt-override.c index 3765a43..7f746ed 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -259,7 +259,7 @@ setPyVirTypedParameter(PyObject *info, return ret;
cleanup: - VIR_FREE(ret); + virTypedParamsFree(ret, size); return NULL; }
-- 1.9.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Mar 18, 2014 at 09:26:08AM +0100, Michal Privoznik wrote:
In the setPyVirTypedParameter we try to produce virTypedParameter array from a pyhthon dictionary. However, when copying field name into
s/pyhthon/python/
item in returned array, we use strncpy() as the field name is fixed length array. To determine its size we use sizeof() but mistakenly dereference it resulting in sizeof(char) which equals to 1 byte. Moreover, there's no need for using sizeof() when we have a global macro to tell us the length of the field name: VIR_TYPED_PARAM_FIELD_LENGTH.
And since array is allocated using VIR_ALLOC() we are sure the memory is initially filled with zeros. Hence, there's no need to terminate string we've just copied into field name with '\0' character. It's there for sure too as we copy up to field length - 1.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
ACK. Martin
--- libvirt-override.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/libvirt-override.c b/libvirt-override.c index 83369fc..3765a43 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -197,8 +197,7 @@ setPyVirTypedParameter(PyObject *info, goto cleanup; }
- strncpy(temp->field, keystr, sizeof(*temp->field) - 1); - temp->field[sizeof(*temp->field) - 1] = '\0'; + strncpy(temp->field, keystr, VIR_TYPED_PARAM_FIELD_LENGTH - 1); temp->type = params[i].type; VIR_FREE(keystr);
-- 1.9.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
Martin Kletzander -
Michal Privoznik