Martin Furter <mfurter_at_bluewin.ch> writes:
> On 12/09/13 15:27, Philip Martin wrote:
>> Martin Furter<mfurter_at_bluewin.ch> writes:
>>
>>> Wouldn't last_dot[1] be more readable than (*(last_dot + 1) ?
>>
>> Probably. I approve a patch if you want to commit.
>
> A quick grep shows 21 of those constructs in the following files:
>
> ./subversion/libsvn_subr/path.c
> ./subversion/libsvn_subr/dirent_uri.c
> ./subversion/libsvn_subr/string.c
> ./subversion/libsvn_ra_svn/marshal.c
> ./subversion/libsvn_diff/diff_memory.c
> ./subversion/libsvn_diff/parse-diff.c
> ./subversion/libsvn_client/patch.c
>
> A grep for "foo[N]" where N is >0 finds more than 900 lines. And lines containing "char foo[N]" are already subtracted.
>
> I guess we should make all these 21 cases consistent with the rest of
> the code.
It depends on how one interprets "consistent with the rest of the
code".
I don't think we should make a global change like the on you propose.
Both constructs are perfectly valid and I see no reason to try to outlaw
one.
>
> [[[
> Replace 21 occurrences of *(foo+N) by the more readable foo[N] syntax.
>
> * subversion/libsvn_client/patch.c
> (readline_prop): Replace *(foo+N) by foo[N].
> * subversion/libsvn_diff/diff_memory.c
> (fill_source_tokens): Ditto.
> * subversion/libsvn_diff/parse-diff.c
> (git_start): Ditto.
> * subversion/libsvn_ra_svn/marshal.c
> (vwrite_tuple): Ditto.
> * subversion/libsvn_subr/dirent_uri.c
> (canonicalize,svn_uri_is_canonical): Ditto.
> * subversion/libsvn_subr/path.c
> (svn_path_splitext): Ditto.
> * subversion/libsvn_subr/string.c
> (svn_cstring_count_newlines): Ditto.
> ]]]
>
> Trying to get it to compile and test but Schwurblix always decides to
> not install the important packages. I'm working on it...
>
> - Martin
>
> Index: subversion/libsvn_client/patch.c
> ===================================================================
> --- subversion/libsvn_client/patch.c (revision 1549528)
> +++ subversion/libsvn_client/patch.c (working copy)
> @@ -568,7 +568,7 @@
> else if (*c == '\r')
> {
> *eol_str = "\r";
> - if (*(c + 1) == '\n')
> + if (c[1] == '\n')
> {
> *eol_str = "\r\n";
> b->offset++;
Take the case above, the surrounding code uses '*c' in many places so
I'd argue that '*(c+1)' is consistent and that 'c[1]' is not an
improvement.
--
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
Received on 2013-12-09 12:43:59 CET