[Libvir] PATCH: const-correct sexpr

[Daniel P. Berrange]

On Mon, Jan 21, 2008 at 12:07:36PM +0100, Jim Meyering wrote:
> "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,

Thanks, that makes sense now - that's a use case I hadn't thought of. No
objections to the patch from me.

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=|