"Daniel P. Berrange" <berrange@xxxxxxxxxx> 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. -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list