[OPEN-ILS-DEV] osrf_json_object.c: const-correctness

[Scott McKellar]

No patch this time -- just a proposal for some patches.

The function jsonObjectGetKey() bothers me because, conceptually, it
is not const-correct.  It receives a pointer to a const jsonObject
and returns a non-const pointer to something inside of the jsonObject.
Using the returned pointer, the calling code can then modify the
contents of the supposedly const jsonObject.

The compiler doesn't object because the function does not modify any
of the members of the jsonObject.  Conceptually, however. the const
pointer points merely to the outermost layer of a data structure that
should be regarded as const in its entirety.  If a function has a 
pointer to a const object, it should not be able to modify the innards 
of the object, at least not without an explicit cast somewhere to 
remove the constness.

As it stands, jsonObjectGetKey() does an end run around the compiler,
and invites conceptual violation of constness, possibly by accident.

In fact there are cases where the calling program uses the returned
pointer to modify the contents of the jsonObject.  I don't think the
jsonObject in question is ever actually const, but the existing
interface is a standing invitation to muddlement.

How to fix?

If this were C++, I could create two overloaded functions that 
differed only in their constness.  One would receive a const pointer 
and return a const pointer, and the other would receive a non-const 
pointer and return a non-const pointer.  The compiler would 
automatically pick the right one according to constness, and enforce 
constness accordingly.

Alas, this is C, and we can't have overloaded functions.  However I
can do the equivalent by creating two nearly identical functions
with different names.  Then I can modify the calling code as needed
to call the right function in each case.

I propose a three-step program:

1. Clone jsonObjectGetKey() to form jsonObjectGetKeyConst().  This
new function would be identical to jsonObjectGetKey() except that it
would return a const pointer.

2. Identify all the places that currently call jsonObjectGetKey().
Wherever the code passes a pointer to a const jsonObject, call the
new function instead.  It will probably be necessary to make some
other changes along the way to keep everything const-correct.

3. Change the original function so that it receives a pointer to a
non-const jsonObject.  Then the compiler won't let you pass a
const pointer to it without a cast.

-----------

Similar considerations apply to jsonObjectGetString(), which returns
a non-const pointer to the string held by a const JSON_STRING.  The
calling code could use this pointer to change the contents of the
supposedly const object.  I suspect that this doesn't ever actually 
happen, in which case we can simply add constness to the return type.
If it does happen, we can use the clone-and-retrofit approach outlined
above.

-----------

Likewise jsonObjectGetIndex() returns a non-const pointer to a
jsonObject inside a supposedly const jsonObject.  Without further
research I don't know if we need to clone-and-retrofit or if we can
just add constness to the return type.

------------

Before wading into the drudgery of creating patches for conceptual
const-correctness, I wanted to get some feedback.  Maybe you'd just
as well leave things alone.

Scott McKellar
http://home.swbell.net/mck9/ct/