[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v4 2/8] virjson: Resolve const correctness

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 redhat com>
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,

-virJSONValueObjectHasKey(virJSONValuePtr object,
+virJSONValueObjectHasKey(const virJSONValue *object,
                         const char *key)
    size_t i;
@@ -753,7 +753,7 @@ virJSONValueObjectHasKey(virJSONValuePtr object,

-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

Attachment: signature.asc
Description: Digital signature

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]