On Thu, Mar 03, 2016 at 06:11:40PM +0100, Michal Privoznik wrote:
Plenty of our virJSON*() APIs don't modify passed object. They merely get a value stored in it. Note this fact in their definition and enforce const correctness. Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> --- src/util/virjson.c | 58 +++++++++++++++++++++++++++--------------------------- src/util/virjson.h | 54 +++++++++++++++++++++++++------------------------- 2 files changed, 56 insertions(+), 56 deletions(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index ae6362b..0a595b9 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -735,7 +735,7 @@ virJSONValueArrayAppend(virJSONValuePtr array, int -virJSONValueObjectHasKey(virJSONValuePtr object, +virJSONValueObjectHasKey(const virJSONValue *object, const char *key) { size_t i; @@ -753,7 +753,7 @@ virJSONValueObjectHasKey(virJSONValuePtr object, virJSONValuePtr -virJSONValueObjectGet(virJSONValuePtr object, +virJSONValueObjectGet(const virJSONValue *object, const char *key)
You're giving the function a const pointer, but it gives you back non-const pointer to a data in that object. That is because the only const part is the above object, however all the data in it (all pointers) are non-const. The function prototype might create false sense of security (the user will think it does not modify the contents, but it can modify most of it). This is one of the few exemptions in which I would rather pass a non-const pointer just so someone (who does not know how the objects are stored internally) is not fooled into thinking it will not do anything with the data. It might cause a problem when they think the returning value is not connected to the object and they can modify it without changing the object because the function they used took just const. Don't take me wrong, I'm all for const-correctness, but to any rule there has to be an exception, right? I would rather make const only those that are not the exception, like virJSONValueObjectGet{Key,String}(), virJSONValueObjectKeysNumber() and so on, but not every one that works. ACK to those from this list that return non-pointers (like int) or const pointers.
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list