Re: CVS update: subversion/subversion/libsvn_subr svn_string.c

Greg Hudson wrote:
>
> > I'd go as far as adding a note to the HACKING file that Subversion
> > code should have no typecasts whatsoever
>
> Well, I sometimes cast a char * to an (unsigned char *) or a char to
> an unsigned char when I'm going to be doing bitwise manipulation on
> the data. I don't think you can get away with no typecasts at all.

O.K., maybe you can't avoid the cast in this case -- although you could
argue that the pointer should've been (unsigned char*) in the first
place.

(Guess, guess: you're doing this in the svndiff (en|de)coder, where
you're
manipulating raw memory. The C standard says that the only portable way
to manipulate raw memory is via (unsigned char*), don't y'know. :-) )

> Some of our code does make a habit of explicitly casting from void *
> to other types (I've been inconsistent myself). I believe that may be
> necessary if you want to be able to cleanly compile the C code as C++
> code, but, I'm not sure.

That's so -- foo* -> void* is implicit, void* -> foo* requires a cast in
C++, though not in C. But we decided some time ago that we don't want to
do this (see the archives -- I opened that discussion, asking about `__'
in identifiers;

    http://subversion.tigris.org/subversion-dev/current/msg00014.html),

that only the public interface should be C++-clean so that SVN clients
can be written in C++.

Well, anyway, I'm not fanatically opposed to casts -- sometimes they're
necessary -- I'm concerned about them for two reasons:

    a) Casts can hide bugs. Even worse, they can prevent the compiler
       from warning about problematic changes that are introduced a long
       time after the cast was written.

    b) It's almost impossible to find them in C. Try grepping for a cast
       some day. :-(

At the very least I'd like to make everyone stop and think before
writing
a cast by putting a

    #define SVN_CAST(TYPE) (TYPE)

somewhere in the headers and decreeing that this is the only way to do
typecasts and furthermore thou shalt comment each typecast thou writest,
on pain of pain.

Worried about how to enforce that rule? That's what
cvs@subversion.tigris.org
is for. Peer pressure, that's the ticket! :-)

(If you guessed from the above that I've been bitten by unnecessary
casts
that were correct when they were put in the code, you guessed correctly.
They really are a bore to debug.)

P.S.: This is my last message for about a week, as I'll be totally
      out of things until the 21st. Which is good, I won't have to
      answer all the flames this'll generate. :-)

-- 
Brane �ibej
  home:   <brane_at_xbc.nu>            http://www.xbc.nu/brane/
   ACM:   <brane_at_acm.org>           http://www.acm.org/