On 13 October 2016 at 15:46, Patrick Steinhardt
<patrick.steinhardt_at_elegosoft.com> wrote:
> Hi,
>
> here's the second version of the conflict option label patch.
> Changes:
>
> - reworded some labels
> - now using apr_array to pass around options
> - renamed and simplified svn_client_resolver_option_label
>
> The functionality has been lightly tested by creating conflict
> scenarios.
>
Quick review:
> + * Return a textual human-readable label of @a option, allocated in
> + * @a result_pool. The label is encoded in UTF-8 and usually
> + * contains up to three words.
> + *
> + * Additionally, the label may be localized to the language used
> + * by the current locale.
> + *
> + * @since New in 1.10.
> + */
> +const char *
> +svn_client_conflict_option_get_label(svn_client_conflict_option_t *option);
The docstring mentions RESULT_POOL, but there is no such argument. I
think it would be better to RESULT_POOL for this function. This would
help to avoid slightly incorrect pool usage like in
find_option_by_builtin():
[[[
client_opt = apr_pcalloc(result_pool, sizeof(*client_opt));
client_opt->choice = id;
client_opt->code = opt->code;
client_opt->label = svn_client_conflict_option_get_label(
builtin_option);
^^^^^ the label is not copied to RESULT_POOL.
SVN_ERR(svn_client_conflict_option_describe(&client_opt->long_desc,
builtin_option,
result_pool,
scratch_pool));
]]]
--
Ivan Zhakov
Received on 2016-10-13 17:08:13 CEST