Stefan Sperling <stsp_at_elego.de> writes:
> > + if (utf8_target[j] == '@')
> > + {
> > + peg_start = utf8_target + j;
>
> Stylistic nit:
>
> With casting added, this becomes:
>
> peg_start = utf8_target + (const char *)j;
>
> This made me scratch my head at first, I hadn't seen pointers
> and ints being mixed like that in a while.
>
> I bet there's tons of code out there that does this, but
> I myself would tend to use an apr_size_t variable initialised
> to strlen(utf8_target), and decrement that in the loop instead of
> adding pointers and ints. That is, I'd try not to rely on automatic
> type conversion too much. See below for an example.
I'd still like to do a real review of the patch, but just regarding
this: FWIW, I think we use pointer arithmetic elsewhere in Subversion,
and I certainly don't mind. It's a completely reliable feature of C, I
think it makes the code more readable rather than less... I don't really
feel like it's "type mixing". Instead, It's just a way of writing an
array reference so that you get the address rather than the value.
Totally your call, of course, Troy.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-05-12 20:16:45 CEST