[libvirt] [PATCH 0/5] fix multiple leasehelper bugs
Found while reviewing Pavel's patch. Peter Krempa (5): util: json: Unify function header formatting util: json: Add helpers for manipulating json arrays net: leasehelper: Don't crash if DNSMASQ doesn't provide lease expiry leasehelper: Ignore corrupted lease file and rewrite it leasehelper: Refactor copying of old entries to avoid double free src/network/leaseshelper.c | 90 +++++++------ src/util/virjson.c | 316 ++++++++++++++++++++++++++++++++++----------- src/util/virjson.h | 2 + 3 files changed, 295 insertions(+), 113 deletions(-) -- 1.9.3
Use consistent formatting of function headers: - two newlines separating functions - function return type on separate line - one argument per line --- src/util/virjson.c | 287 +++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 215 insertions(+), 72 deletions(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index 35a8252..af760a8 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -63,7 +63,8 @@ struct _virJSONParser { }; -void virJSONValueFree(virJSONValuePtr value) +void +virJSONValueFree(virJSONValuePtr value) { size_t i; if (!value || value->protect) @@ -97,7 +98,8 @@ void virJSONValueFree(virJSONValuePtr value) } -virJSONValuePtr virJSONValueNewString(const char *data) +virJSONValuePtr +virJSONValueNewString(const char *data) { virJSONValuePtr val; @@ -116,7 +118,9 @@ virJSONValuePtr virJSONValueNewString(const char *data) return val; } -virJSONValuePtr virJSONValueNewStringLen(const char *data, size_t length) + +virJSONValuePtr +virJSONValueNewStringLen(const char *data, size_t length) { virJSONValuePtr val; @@ -135,7 +139,9 @@ virJSONValuePtr virJSONValueNewStringLen(const char *data, size_t length) return val; } -static virJSONValuePtr virJSONValueNewNumber(const char *data) + +static virJSONValuePtr +virJSONValueNewNumber(const char *data) { virJSONValuePtr val; @@ -151,7 +157,9 @@ static virJSONValuePtr virJSONValueNewNumber(const char *data) return val; } -virJSONValuePtr virJSONValueNewNumberInt(int data) + +virJSONValuePtr +virJSONValueNewNumberInt(int data) { virJSONValuePtr val = NULL; char *str; @@ -163,7 +171,8 @@ virJSONValuePtr virJSONValueNewNumberInt(int data) } -virJSONValuePtr virJSONValueNewNumberUint(unsigned int data) +virJSONValuePtr +virJSONValueNewNumberUint(unsigned int data) { virJSONValuePtr val = NULL; char *str; @@ -175,7 +184,8 @@ virJSONValuePtr virJSONValueNewNumberUint(unsigned int data) } -virJSONValuePtr virJSONValueNewNumberLong(long long data) +virJSONValuePtr +virJSONValueNewNumberLong(long long data) { virJSONValuePtr val = NULL; char *str; @@ -187,7 +197,8 @@ virJSONValuePtr virJSONValueNewNumberLong(long long data) } -virJSONValuePtr virJSONValueNewNumberUlong(unsigned long long data) +virJSONValuePtr +virJSONValueNewNumberUlong(unsigned long long data) { virJSONValuePtr val = NULL; char *str; @@ -199,7 +210,8 @@ virJSONValuePtr virJSONValueNewNumberUlong(unsigned long long data) } -virJSONValuePtr virJSONValueNewNumberDouble(double data) +virJSONValuePtr +virJSONValueNewNumberDouble(double data) { virJSONValuePtr val = NULL; char *str; @@ -211,7 +223,8 @@ virJSONValuePtr virJSONValueNewNumberDouble(double data) } -virJSONValuePtr virJSONValueNewBoolean(int boolean_) +virJSONValuePtr +virJSONValueNewBoolean(int boolean_) { virJSONValuePtr val; @@ -224,7 +237,9 @@ virJSONValuePtr virJSONValueNewBoolean(int boolean_) return val; } -virJSONValuePtr virJSONValueNewNull(void) + +virJSONValuePtr +virJSONValueNewNull(void) { virJSONValuePtr val; @@ -236,7 +251,9 @@ virJSONValuePtr virJSONValueNewNull(void) return val; } -virJSONValuePtr virJSONValueNewArray(void) + +virJSONValuePtr +virJSONValueNewArray(void) { virJSONValuePtr val; @@ -248,7 +265,9 @@ virJSONValuePtr virJSONValueNewArray(void) return val; } -virJSONValuePtr virJSONValueNewObject(void) + +virJSONValuePtr +virJSONValueNewObject(void) { virJSONValuePtr val; @@ -260,7 +279,11 @@ virJSONValuePtr virJSONValueNewObject(void) return val; } -int virJSONValueObjectAppend(virJSONValuePtr object, const char *key, virJSONValuePtr value) + +int +virJSONValueObjectAppend(virJSONValuePtr object, + const char *key, + virJSONValuePtr value) { char *newkey; @@ -287,7 +310,10 @@ int virJSONValueObjectAppend(virJSONValuePtr object, const char *key, virJSONVal } -int virJSONValueObjectAppendString(virJSONValuePtr object, const char *key, const char *value) +int +virJSONValueObjectAppendString(virJSONValuePtr object, + const char *key, + const char *value) { virJSONValuePtr jvalue = virJSONValueNewString(value); if (!jvalue) @@ -299,7 +325,11 @@ int virJSONValueObjectAppendString(virJSONValuePtr object, const char *key, cons return 0; } -int virJSONValueObjectAppendNumberInt(virJSONValuePtr object, const char *key, int number) + +int +virJSONValueObjectAppendNumberInt(virJSONValuePtr object, + const char *key, + int number) { virJSONValuePtr jvalue = virJSONValueNewNumberInt(number); if (!jvalue) @@ -312,7 +342,10 @@ int virJSONValueObjectAppendNumberInt(virJSONValuePtr object, const char *key, i } -int virJSONValueObjectAppendNumberUint(virJSONValuePtr object, const char *key, unsigned int number) +int +virJSONValueObjectAppendNumberUint(virJSONValuePtr object, + const char *key, + unsigned int number) { virJSONValuePtr jvalue = virJSONValueNewNumberUint(number); if (!jvalue) @@ -324,7 +357,11 @@ int virJSONValueObjectAppendNumberUint(virJSONValuePtr object, const char *key, return 0; } -int virJSONValueObjectAppendNumberLong(virJSONValuePtr object, const char *key, long long number) + +int +virJSONValueObjectAppendNumberLong(virJSONValuePtr object, + const char *key, + long long number) { virJSONValuePtr jvalue = virJSONValueNewNumberLong(number); if (!jvalue) @@ -336,7 +373,11 @@ int virJSONValueObjectAppendNumberLong(virJSONValuePtr object, const char *key, return 0; } -int virJSONValueObjectAppendNumberUlong(virJSONValuePtr object, const char *key, unsigned long long number) + +int +virJSONValueObjectAppendNumberUlong(virJSONValuePtr object, + const char *key, + unsigned long long number) { virJSONValuePtr jvalue = virJSONValueNewNumberUlong(number); if (!jvalue) @@ -348,7 +389,11 @@ int virJSONValueObjectAppendNumberUlong(virJSONValuePtr object, const char *key, return 0; } -int virJSONValueObjectAppendNumberDouble(virJSONValuePtr object, const char *key, double number) + +int +virJSONValueObjectAppendNumberDouble(virJSONValuePtr object, + const char *key, + double number) { virJSONValuePtr jvalue = virJSONValueNewNumberDouble(number); if (!jvalue) @@ -360,7 +405,11 @@ int virJSONValueObjectAppendNumberDouble(virJSONValuePtr object, const char *key return 0; } -int virJSONValueObjectAppendBoolean(virJSONValuePtr object, const char *key, int boolean_) + +int +virJSONValueObjectAppendBoolean(virJSONValuePtr object, + const char *key, + int boolean_) { virJSONValuePtr jvalue = virJSONValueNewBoolean(boolean_); if (!jvalue) @@ -372,7 +421,10 @@ int virJSONValueObjectAppendBoolean(virJSONValuePtr object, const char *key, int return 0; } -int virJSONValueObjectAppendNull(virJSONValuePtr object, const char *key) + +int +virJSONValueObjectAppendNull(virJSONValuePtr object, + const char *key) { virJSONValuePtr jvalue = virJSONValueNewNull(); if (!jvalue) @@ -385,7 +437,9 @@ int virJSONValueObjectAppendNull(virJSONValuePtr object, const char *key) } -int virJSONValueArrayAppend(virJSONValuePtr array, virJSONValuePtr value) +int +virJSONValueArrayAppend(virJSONValuePtr array, + virJSONValuePtr value) { if (array->type != VIR_JSON_TYPE_ARRAY) return -1; @@ -400,7 +454,10 @@ int virJSONValueArrayAppend(virJSONValuePtr array, virJSONValuePtr value) return 0; } -int virJSONValueObjectHasKey(virJSONValuePtr object, const char *key) + +int +virJSONValueObjectHasKey(virJSONValuePtr object, + const char *key) { size_t i; @@ -415,7 +472,10 @@ int virJSONValueObjectHasKey(virJSONValuePtr object, const char *key) return 0; } -virJSONValuePtr virJSONValueObjectGet(virJSONValuePtr object, const char *key) + +virJSONValuePtr +virJSONValueObjectGet(virJSONValuePtr object, + const char *key) { size_t i; @@ -430,7 +490,9 @@ virJSONValuePtr virJSONValueObjectGet(virJSONValuePtr object, const char *key) return NULL; } -int virJSONValueObjectKeysNumber(virJSONValuePtr object) + +int +virJSONValueObjectKeysNumber(virJSONValuePtr object) { if (object->type != VIR_JSON_TYPE_OBJECT) return -1; @@ -438,7 +500,10 @@ int virJSONValueObjectKeysNumber(virJSONValuePtr object) return object->data.object.npairs; } -const char *virJSONValueObjectGetKey(virJSONValuePtr object, unsigned int n) + +const char * +virJSONValueObjectGetKey(virJSONValuePtr object, + unsigned int n) { if (object->type != VIR_JSON_TYPE_OBJECT) return NULL; @@ -449,11 +514,13 @@ const char *virJSONValueObjectGetKey(virJSONValuePtr object, unsigned int n) return object->data.object.pairs[n].key; } + /* Remove the key-value pair tied to @key out of @object. If @value is * not NULL, the dropped value object is returned instead of freed. * Returns 1 on success, 0 if no key was found, and -1 on error. */ int -virJSONValueObjectRemoveKey(virJSONValuePtr object, const char *key, +virJSONValueObjectRemoveKey(virJSONValuePtr object, + const char *key, virJSONValuePtr *value) { size_t i; @@ -481,7 +548,10 @@ virJSONValueObjectRemoveKey(virJSONValuePtr object, const char *key, return 0; } -virJSONValuePtr virJSONValueObjectGetValue(virJSONValuePtr object, unsigned int n) + +virJSONValuePtr +virJSONValueObjectGetValue(virJSONValuePtr object, + unsigned int n) { if (object->type != VIR_JSON_TYPE_OBJECT) return NULL; @@ -492,7 +562,9 @@ virJSONValuePtr virJSONValueObjectGetValue(virJSONValuePtr object, unsigned int return object->data.object.pairs[n].value; } -int virJSONValueArraySize(virJSONValuePtr array) + +int +virJSONValueArraySize(virJSONValuePtr array) { if (array->type != VIR_JSON_TYPE_ARRAY) return -1; @@ -501,7 +573,9 @@ int virJSONValueArraySize(virJSONValuePtr array) } -virJSONValuePtr virJSONValueArrayGet(virJSONValuePtr array, unsigned int element) +virJSONValuePtr +virJSONValueArrayGet(virJSONValuePtr array, + unsigned int element) { if (array->type != VIR_JSON_TYPE_ARRAY) return NULL; @@ -512,7 +586,9 @@ virJSONValuePtr virJSONValueArrayGet(virJSONValuePtr array, unsigned int element return array->data.array.values[element]; } -const char *virJSONValueGetString(virJSONValuePtr string) + +const char * +virJSONValueGetString(virJSONValuePtr string) { if (string->type != VIR_JSON_TYPE_STRING) return NULL; @@ -521,7 +597,9 @@ const char *virJSONValueGetString(virJSONValuePtr string) } -int virJSONValueGetNumberInt(virJSONValuePtr number, int *value) +int +virJSONValueGetNumberInt(virJSONValuePtr number, + int *value) { if (number->type != VIR_JSON_TYPE_NUMBER) return -1; @@ -529,7 +607,10 @@ int virJSONValueGetNumberInt(virJSONValuePtr number, int *value) return virStrToLong_i(number->data.number, NULL, 10, value); } -int virJSONValueGetNumberUint(virJSONValuePtr number, unsigned int *value) + +int +virJSONValueGetNumberUint(virJSONValuePtr number, + unsigned int *value) { if (number->type != VIR_JSON_TYPE_NUMBER) return -1; @@ -537,7 +618,10 @@ int virJSONValueGetNumberUint(virJSONValuePtr number, unsigned int *value) return virStrToLong_ui(number->data.number, NULL, 10, value); } -int virJSONValueGetNumberLong(virJSONValuePtr number, long long *value) + +int +virJSONValueGetNumberLong(virJSONValuePtr number, + long long *value) { if (number->type != VIR_JSON_TYPE_NUMBER) return -1; @@ -545,7 +629,10 @@ int virJSONValueGetNumberLong(virJSONValuePtr number, long long *value) return virStrToLong_ll(number->data.number, NULL, 10, value); } -int virJSONValueGetNumberUlong(virJSONValuePtr number, unsigned long long *value) + +int +virJSONValueGetNumberUlong(virJSONValuePtr number, + unsigned long long *value) { if (number->type != VIR_JSON_TYPE_NUMBER) return -1; @@ -553,7 +640,10 @@ int virJSONValueGetNumberUlong(virJSONValuePtr number, unsigned long long *value return virStrToLong_ull(number->data.number, NULL, 10, value); } -int virJSONValueGetNumberDouble(virJSONValuePtr number, double *value) + +int +virJSONValueGetNumberDouble(virJSONValuePtr number, + double *value) { if (number->type != VIR_JSON_TYPE_NUMBER) return -1; @@ -562,7 +652,9 @@ int virJSONValueGetNumberDouble(virJSONValuePtr number, double *value) } -int virJSONValueGetBoolean(virJSONValuePtr val, bool *value) +int +virJSONValueGetBoolean(virJSONValuePtr val, + bool *value) { if (val->type != VIR_JSON_TYPE_BOOLEAN) return -1; @@ -572,7 +664,8 @@ int virJSONValueGetBoolean(virJSONValuePtr val, bool *value) } -int virJSONValueIsNull(virJSONValuePtr val) +int +virJSONValueIsNull(virJSONValuePtr val) { if (val->type != VIR_JSON_TYPE_NULL) return 0; @@ -581,7 +674,9 @@ int virJSONValueIsNull(virJSONValuePtr val) } -const char *virJSONValueObjectGetString(virJSONValuePtr object, const char *key) +const char * +virJSONValueObjectGetString(virJSONValuePtr object, + const char *key) { virJSONValuePtr val; if (object->type != VIR_JSON_TYPE_OBJECT) @@ -595,7 +690,10 @@ const char *virJSONValueObjectGetString(virJSONValuePtr object, const char *key) } -int virJSONValueObjectGetNumberInt(virJSONValuePtr object, const char *key, int *value) +int +virJSONValueObjectGetNumberInt(virJSONValuePtr object, + const char *key, + int *value) { virJSONValuePtr val; if (object->type != VIR_JSON_TYPE_OBJECT) @@ -609,7 +707,10 @@ int virJSONValueObjectGetNumberInt(virJSONValuePtr object, const char *key, int } -int virJSONValueObjectGetNumberUint(virJSONValuePtr object, const char *key, unsigned int *value) +int +virJSONValueObjectGetNumberUint(virJSONValuePtr object, + const char *key, + unsigned int *value) { virJSONValuePtr val; if (object->type != VIR_JSON_TYPE_OBJECT) @@ -623,7 +724,10 @@ int virJSONValueObjectGetNumberUint(virJSONValuePtr object, const char *key, uns } -int virJSONValueObjectGetNumberLong(virJSONValuePtr object, const char *key, long long *value) +int +virJSONValueObjectGetNumberLong(virJSONValuePtr object, + const char *key, + long long *value) { virJSONValuePtr val; if (object->type != VIR_JSON_TYPE_OBJECT) @@ -637,7 +741,10 @@ int virJSONValueObjectGetNumberLong(virJSONValuePtr object, const char *key, lon } -int virJSONValueObjectGetNumberUlong(virJSONValuePtr object, const char *key, unsigned long long *value) +int +virJSONValueObjectGetNumberUlong(virJSONValuePtr object, + const char *key, + unsigned long long *value) { virJSONValuePtr val; if (object->type != VIR_JSON_TYPE_OBJECT) @@ -651,7 +758,10 @@ int virJSONValueObjectGetNumberUlong(virJSONValuePtr object, const char *key, un } -int virJSONValueObjectGetNumberDouble(virJSONValuePtr object, const char *key, double *value) +int +virJSONValueObjectGetNumberDouble(virJSONValuePtr object, + const char *key, + double *value) { virJSONValuePtr val; if (object->type != VIR_JSON_TYPE_OBJECT) @@ -665,7 +775,10 @@ int virJSONValueObjectGetNumberDouble(virJSONValuePtr object, const char *key, d } -int virJSONValueObjectGetBoolean(virJSONValuePtr object, const char *key, bool *value) +int +virJSONValueObjectGetBoolean(virJSONValuePtr object, + const char *key, + bool *value) { virJSONValuePtr val; if (object->type != VIR_JSON_TYPE_OBJECT) @@ -679,7 +792,9 @@ int virJSONValueObjectGetBoolean(virJSONValuePtr object, const char *key, bool * } -int virJSONValueObjectIsNull(virJSONValuePtr object, const char *key) +int +virJSONValueObjectIsNull(virJSONValuePtr object, + const char *key) { virJSONValuePtr val; if (object->type != VIR_JSON_TYPE_OBJECT) @@ -694,8 +809,9 @@ int virJSONValueObjectIsNull(virJSONValuePtr object, const char *key) #if WITH_YAJL -static int virJSONParserInsertValue(virJSONParserPtr parser, - virJSONValuePtr value) +static int +virJSONParserInsertValue(virJSONParserPtr parser, + virJSONValuePtr value) { if (!parser->head) { parser->head = value; @@ -743,7 +859,9 @@ static int virJSONParserInsertValue(virJSONParserPtr parser, return 0; } -static int virJSONParserHandleNull(void *ctx) + +static int +virJSONParserHandleNull(void *ctx) { virJSONParserPtr parser = ctx; virJSONValuePtr value = virJSONValueNewNull(); @@ -761,7 +879,10 @@ static int virJSONParserHandleNull(void *ctx) return 1; } -static int virJSONParserHandleBoolean(void *ctx, int boolean_) + +static int +virJSONParserHandleBoolean(void *ctx, + int boolean_) { virJSONParserPtr parser = ctx; virJSONValuePtr value = virJSONValueNewBoolean(boolean_); @@ -779,9 +900,11 @@ static int virJSONParserHandleBoolean(void *ctx, int boolean_) return 1; } -static int virJSONParserHandleNumber(void *ctx, - const char *s, - yajl_size_t l) + +static int +virJSONParserHandleNumber(void *ctx, + const char *s, + yajl_size_t l) { virJSONParserPtr parser = ctx; char *str; @@ -805,9 +928,11 @@ static int virJSONParserHandleNumber(void *ctx, return 1; } -static int virJSONParserHandleString(void *ctx, - const unsigned char *stringVal, - yajl_size_t stringLen) + +static int +virJSONParserHandleString(void *ctx, + const unsigned char *stringVal, + yajl_size_t stringLen) { virJSONParserPtr parser = ctx; virJSONValuePtr value = virJSONValueNewStringLen((const char *)stringVal, @@ -826,9 +951,11 @@ static int virJSONParserHandleString(void *ctx, return 1; } -static int virJSONParserHandleMapKey(void *ctx, - const unsigned char *stringVal, - yajl_size_t stringLen) + +static int +virJSONParserHandleMapKey(void *ctx, + const unsigned char *stringVal, + yajl_size_t stringLen) { virJSONParserPtr parser = ctx; virJSONParserStatePtr state; @@ -846,7 +973,9 @@ static int virJSONParserHandleMapKey(void *ctx, return 1; } -static int virJSONParserHandleStartMap(void *ctx) + +static int +virJSONParserHandleStartMap(void *ctx) { virJSONParserPtr parser = ctx; virJSONValuePtr value = virJSONValueNewObject(); @@ -874,7 +1003,8 @@ static int virJSONParserHandleStartMap(void *ctx) } -static int virJSONParserHandleEndMap(void *ctx) +static int +virJSONParserHandleEndMap(void *ctx) { virJSONParserPtr parser = ctx; virJSONParserStatePtr state; @@ -895,7 +1025,9 @@ static int virJSONParserHandleEndMap(void *ctx) return 1; } -static int virJSONParserHandleStartArray(void *ctx) + +static int +virJSONParserHandleStartArray(void *ctx) { virJSONParserPtr parser = ctx; virJSONValuePtr value = virJSONValueNewArray(); @@ -921,7 +1053,9 @@ static int virJSONParserHandleStartArray(void *ctx) return 1; } -static int virJSONParserHandleEndArray(void *ctx) + +static int +virJSONParserHandleEndArray(void *ctx) { virJSONParserPtr parser = ctx; virJSONParserStatePtr state; @@ -942,6 +1076,7 @@ static int virJSONParserHandleEndArray(void *ctx) return 1; } + static const yajl_callbacks parserCallbacks = { virJSONParserHandleNull, virJSONParserHandleBoolean, @@ -958,7 +1093,8 @@ static const yajl_callbacks parserCallbacks = { /* XXX add an incremental streaming parser - yajl trivially supports it */ -virJSONValuePtr virJSONValueFromString(const char *jsonstring) +virJSONValuePtr +virJSONValueFromString(const char *jsonstring) { yajl_handle hand; virJSONParser parser = { NULL, NULL, 0 }; @@ -1024,8 +1160,9 @@ virJSONValuePtr virJSONValueFromString(const char *jsonstring) } -static int virJSONValueToStringOne(virJSONValuePtr object, - yajl_gen g) +static int +virJSONValueToStringOne(virJSONValuePtr object, + yajl_gen g) { size_t i; @@ -1087,8 +1224,10 @@ static int virJSONValueToStringOne(virJSONValuePtr object, return 0; } -char *virJSONValueToString(virJSONValuePtr object, - bool pretty) + +char * +virJSONValueToString(virJSONValuePtr object, + bool pretty) { yajl_gen g; const unsigned char *str; @@ -1138,14 +1277,18 @@ char *virJSONValueToString(virJSONValuePtr object, #else -virJSONValuePtr virJSONValueFromString(const char *jsonstring ATTRIBUTE_UNUSED) +virJSONValuePtr +virJSONValueFromString(const char *jsonstring ATTRIBUTE_UNUSED) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("No JSON parser implementation is available")); return NULL; } -char *virJSONValueToString(virJSONValuePtr object ATTRIBUTE_UNUSED, - bool pretty ATTRIBUTE_UNUSED) + + +char * +virJSONValueToString(virJSONValuePtr object ATTRIBUTE_UNUSED, + bool pretty ATTRIBUTE_UNUSED) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("No JSON parser implementation is available")); -- 1.9.3
On 16.6.2014 17:21, Peter Krempa wrote:
Use consistent formatting of function headers: - two newlines separating functions - function return type on separate line - one argument per line --- src/util/virjson.c | 287 +++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 215 insertions(+), 72 deletions(-)
diff --git a/src/util/virjson.c b/src/util/virjson.c index 35a8252..af760a8 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -63,7 +63,8 @@ struct _virJSONParser { };
-void virJSONValueFree(virJSONValuePtr value) +void +virJSONValueFree(virJSONValuePtr value) { size_t i; if (!value || value->protect) @@ -97,7 +98,8 @@ void virJSONValueFree(virJSONValuePtr value) }
-virJSONValuePtr virJSONValueNewString(const char *data) +virJSONValuePtr +virJSONValueNewString(const char *data) { virJSONValuePtr val;
@@ -116,7 +118,9 @@ virJSONValuePtr virJSONValueNewString(const char *data) return val; }
-virJSONValuePtr virJSONValueNewStringLen(const char *data, size_t length) + +virJSONValuePtr +virJSONValueNewStringLen(const char *data, size_t length)
You've missed one function with two arguments. [..] ACK with the change above. Pavel
Add a checker to determine whether a JSON object is an array and a helper to steal objects from a JSON array. --- src/util/virjson.c | 29 +++++++++++++++++++++++++++++ src/util/virjson.h | 2 ++ 2 files changed, 31 insertions(+) diff --git a/src/util/virjson.c b/src/util/virjson.c index af760a8..005406a 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -563,6 +563,13 @@ virJSONValueObjectGetValue(virJSONValuePtr object, } +bool +virJSONValueIsArray(virJSONValuePtr array) +{ + return array->type == VIR_JSON_TYPE_ARRAY; +} + + int virJSONValueArraySize(virJSONValuePtr array) { @@ -587,6 +594,28 @@ virJSONValueArrayGet(virJSONValuePtr array, } +virJSONValuePtr +virJSONValueArraySteal(virJSONValuePtr array, + unsigned int element) +{ + virJSONValuePtr ret = NULL; + + if (array->type != VIR_JSON_TYPE_ARRAY) + return NULL; + + if (element >= array->data.array.nvalues) + return NULL; + + ret = array->data.array.values[element]; + + VIR_DELETE_ELEMENT(array->data.array.values, + element, + array->data.array.nvalues); + + return ret; +} + + const char * virJSONValueGetString(virJSONValuePtr string) { diff --git a/src/util/virjson.h b/src/util/virjson.h index db11396..9487729 100644 --- a/src/util/virjson.h +++ b/src/util/virjson.h @@ -97,8 +97,10 @@ int virJSONValueArrayAppend(virJSONValuePtr object, virJSONValuePtr value); int virJSONValueObjectHasKey(virJSONValuePtr object, const char *key); virJSONValuePtr virJSONValueObjectGet(virJSONValuePtr object, const char *key); +bool virJSONValueIsArray(virJSONValuePtr array); int virJSONValueArraySize(virJSONValuePtr object); virJSONValuePtr virJSONValueArrayGet(virJSONValuePtr object, unsigned int element); +virJSONValuePtr virJSONValueArraySteal(virJSONValuePtr object, unsigned int element); int virJSONValueObjectKeysNumber(virJSONValuePtr object); const char *virJSONValueObjectGetKey(virJSONValuePtr object, unsigned int n); -- 1.9.3
On 16.6.2014 17:21, Peter Krempa wrote:
Add a checker to determine whether a JSON object is an array and a helper to steal objects from a JSON array. --- src/util/virjson.c | 29 +++++++++++++++++++++++++++++ src/util/virjson.h | 2 ++ 2 files changed, 31 insertions(+)
ACK Pavel
The value is provided via environment and causes a crash if not defined. --- src/network/leaseshelper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c index bf1842b..99c47a9 100644 --- a/src/network/leaseshelper.c +++ b/src/network/leaseshelper.c @@ -181,7 +181,8 @@ main(int argc, char **argv) goto cleanup; /* Removed extraneous trailing space in DNSMASQ_LEASE_EXPIRES (dnsmasq < 2.52) */ - if (exptime[strlen(exptime) - 1] == ' ') + if (exptime && + exptime[strlen(exptime) - 1] == ' ') exptime[strlen(exptime) - 1] = '\0'; /* Check if it is an IPv6 lease */ -- 1.9.3
On 16.6.2014 17:21, Peter Krempa wrote:
The value is provided via environment and causes a crash if not defined. --- src/network/leaseshelper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c index bf1842b..99c47a9 100644 --- a/src/network/leaseshelper.c +++ b/src/network/leaseshelper.c @@ -181,7 +181,8 @@ main(int argc, char **argv) goto cleanup;
/* Removed extraneous trailing space in DNSMASQ_LEASE_EXPIRES (dnsmasq < 2.52) */ - if (exptime[strlen(exptime) - 1] == ' ') + if (exptime && + exptime[strlen(exptime) - 1] == ' ') exptime[strlen(exptime) - 1] = '\0';
/* Check if it is an IPv6 lease */
ACK Pavel
On 16.6.2014 17:40, Pavel Hrdina wrote:
On 16.6.2014 17:21, Peter Krempa wrote:
The value is provided via environment and causes a crash if not defined. --- src/network/leaseshelper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c index bf1842b..99c47a9 100644 --- a/src/network/leaseshelper.c +++ b/src/network/leaseshelper.c @@ -181,7 +181,8 @@ main(int argc, char **argv) goto cleanup;
/* Removed extraneous trailing space in DNSMASQ_LEASE_EXPIRES (dnsmasq < 2.52) */ - if (exptime[strlen(exptime) - 1] == ' ') + if (exptime && + exptime[strlen(exptime) - 1] == ' ') exptime[strlen(exptime) - 1] = '\0';
/* Check if it is an IPv6 lease */
ACK
Pavel
I've missed the s/leasehelper/leaseshelper/ in subject.
Instead of reporting and error and terminating, rewrite the file with the newly learned info. --- src/network/leaseshelper.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c index 99c47a9..d4b48bb 100644 --- a/src/network/leaseshelper.c +++ b/src/network/leaseshelper.c @@ -274,14 +274,14 @@ main(int argc, char **argv) if (custom_lease_file_len) { if (!(leases_array = virJSONValueFromString(lease_entries))) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("invalid json in file: %s"), custom_lease_file); - goto cleanup; - } - - if ((size = virJSONValueArraySize(leases_array)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("couldn't fetch array of leases")); - goto cleanup; + _("invalid json in file: %s, rewriting it"), + custom_lease_file); + } else { + if ((size = virJSONValueArraySize(leases_array)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("couldn't fetch array of leases")); + goto cleanup; + } } } -- 1.9.3
On 16.6.2014 17:21, Peter Krempa wrote: s/leasehelper/leaseshelper/ in subject
Instead of reporting and error and terminating, rewrite the file with the newly learned info.
s/and error/an error/
--- src/network/leaseshelper.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
ACK with changes Pavel
On 06/16/2014 09:21 AM, Peter Krempa wrote:
Instead of reporting and error and terminating, rewrite the file with
s/and/an/
the newly learned info. --- src/network/leaseshelper.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
When copying entries from the old lease file into the new array the old code would copy the pointer of the json object into the second array without removing it from the first. Afterwards when both arrays were freed this might lead to a crash due to access of already freed memory. Refactor the code to use the new array item stealing helper added to the json code so that the entry resides just in one array. --- src/network/leaseshelper.c | 79 +++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 36 deletions(-) diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c index d4b48bb..bb0959c 100644 --- a/src/network/leaseshelper.c +++ b/src/network/leaseshelper.c @@ -116,7 +116,6 @@ main(int argc, char **argv) long long currtime = 0; long long expirytime = 0; size_t i = 0; - int size = 0; int action = -1; int pid_file_fd = -1; int rv = EXIT_FAILURE; @@ -270,6 +269,15 @@ main(int argc, char **argv) virLeaseActionTypeToString(action)); exit(EXIT_FAILURE); } + + if (!(leases_array_new = virJSONValueNewArray())) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to create json")); + goto cleanup; + } + + currtime = (long long) time(NULL); + /* Check for previous leases */ if (custom_lease_file_len) { if (!(leases_array = virJSONValueFromString(lease_entries))) { @@ -277,52 +285,51 @@ main(int argc, char **argv) _("invalid json in file: %s, rewriting it"), custom_lease_file); } else { - if ((size = virJSONValueArraySize(leases_array)) < 0) { + if (!virJSONValueIsArray(leases_array)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("couldn't fetch array of leases")); goto cleanup; } - } - } - - if (!(leases_array_new = virJSONValueNewArray())) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to create json")); - goto cleanup; - } - currtime = (long long) time(NULL); + i = 0; + while (i < virJSONValueArraySize(leases_array)) { + const char *ip_tmp = NULL; + long long expirytime_tmp = -1; - for (i = 0; i < size; i++) { - const char *ip_tmp = NULL; - long long expirytime_tmp = -1; + if (!(lease_tmp = virJSONValueArrayGet(leases_array, i))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse json")); + goto cleanup; + } - if (!(lease_tmp = virJSONValueArrayGet(leases_array, i))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to parse json")); - goto cleanup; - } + if (!(ip_tmp = virJSONValueObjectGetString(lease_tmp, "ip-address")) || + (virJSONValueObjectGetNumberLong(lease_tmp, "expiry-time", &expirytime_tmp) < 0)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse json")); + goto cleanup; + } - if (!(ip_tmp = virJSONValueObjectGetString(lease_tmp, "ip-address")) || - (virJSONValueObjectGetNumberLong(lease_tmp, "expiry-time", &expirytime_tmp) < 0)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to parse json")); - goto cleanup; - } + /* Check whether lease has expired or not */ + if (expirytime_tmp < currtime) { + i++; + continue; + } - /* Check whether lease has expired or not */ - if (expirytime_tmp < currtime) - continue; + /* Check whether lease has to be included or not */ + if (delete && STREQ(ip_tmp, ip)) { + i++; + continue; + } - /* Check whether lease has to be included or not */ - if (delete && STREQ(ip_tmp, ip)) - continue; + /* Move old lease to new array */ + lease_tmp = virJSONValueArraySteal(leases_array, i); - /* Add old lease to new array */ - if (virJSONValueArrayAppend(leases_array_new, lease_tmp) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to create json")); - goto cleanup; + if (virJSONValueArrayAppend(leases_array_new, lease_tmp) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to create json")); + goto cleanup; + } + } } } -- 1.9.3
On 16.6.2014 17:21, Peter Krempa wrote: s/leasehelper/leaseshelper/ in subject
When copying entries from the old lease file into the new array the old code would copy the pointer of the json object into the second array without removing it from the first. Afterwards when both arrays were freed this might lead to a crash due to access of already freed memory.
Refactor the code to use the new array item stealing helper added to the json code so that the entry resides just in one array. --- src/network/leaseshelper.c | 79 +++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 36 deletions(-)
[..]
- /* Check whether lease has to be included or not */ - if (delete && STREQ(ip_tmp, ip)) - continue; + /* Move old lease to new array */ + lease_tmp = virJSONValueArraySteal(leases_array, i);
There is a possible memory leak if the append fails. Probably move this function after the successful append and ignore the return value.
- /* Add old lease to new array */ - if (virJSONValueArrayAppend(leases_array_new, lease_tmp) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to create json")); - goto cleanup; + if (virJSONValueArrayAppend(leases_array_new, lease_tmp) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to create json")); + goto cleanup; + } + } } }
ACK with changes Pavel
On 06/16/14 18:00, Pavel Hrdina wrote:
On 16.6.2014 17:21, Peter Krempa wrote:
s/leasehelper/leaseshelper/ in subject
When copying entries from the old lease file into the new array the old code would copy the pointer of the json object into the second array without removing it from the first. Afterwards when both arrays were freed this might lead to a crash due to access of already freed memory.
Refactor the code to use the new array item stealing helper added to the json code so that the entry resides just in one array. --- src/network/leaseshelper.c | 79 +++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 36 deletions(-)
ACK with changes
Thanks, I've fixed the nits and pushed the series.
participants (3)
-
Eric Blake -
Pavel Hrdina -
Peter Krempa