[Libvir] PATCH: const-correct sexpr
Jim Meyering
jim at meyering.net
Mon Jan 21 11:07:36 UTC 2008
"Daniel P. Berrange" <berrange at 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.
More information about the libvir-list
mailing list