Re: [Libvir] PATCH: const-correct sexpr
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Sat, Jan 19, 2008 at 10:48:46PM +0100, Jim Meyering wrote:
> I was looking at xend_internal.c and wondered why sexpr_int's
> sexpr pointer wasn't const... surely, it *can't* modify that, I thought.
> So I made it const, and pulled the thread, which ended up making most
> of the sexpr* parameters in sexpr.[ch] const.
>
> I added the few inevitable casts, but imho, that cost is far
> outweighed by having accurate prototypes for all of these functions.
I agree with the lookup/getter related functions being made const, but is
it sensible to make the constructor parms const too ? I mean these two
methods:
Yes. See below.
> -struct sexpr *sexpr_cons(struct sexpr *car, struct sexpr *cdr);
> -struct sexpr *sexpr_append(struct sexpr *lst, struct sexpr *item);
> +struct sexpr *sexpr_cons(const struct sexpr *car, const struct sexpr *cdr);
> +struct sexpr *sexpr_append(struct sexpr *lst, const struct sexpr *item);
The parameters passed in here, will be 'owned' by the resulting
non-const sexpr that is constructed. The params are not copied
during the constructor, so to my mind they should not be const.
They need to be allocated by the caller, and ownership is taken
by the new sexpr.
My personal rule for when to make a pointer parameter P "const" is to
do it whenever neither the function in question nor any of its callees
modifies the data behind the pointer. This permits a cast-free call to
the function with a parameter that is a "const" pointer. In this case,
that argument pointer should always alias a non-const pointer, but
that's fine, of course.
This is important e.g.,. if I have a function that treats a pair of sexpr
as read-only, and that also happens to concatenate them. For example,
T *
something_read_only (const T *t, const T *u)
{
T *tmp = sexpr_cons (t, u);
// operate on TMP
...
}
If the sexpr_cons parameters are not declared "const", then that
forces the developer to choose between two poor alternatives:
* use an inaccurate (non-const-correct) interface for the calling function
* use casts to avoid the resulting warnings
T *t = ...
T *u = ...
...
const T *tp = t;
// do read-only things via "tp"
...
T *s = sexpr_cons (tp, u);
I.e., if a function like sexpr_cons takes non-const
parameters, then it mandates a cast-away-const for any
use like this one.