Thanks, r1073377. See below for some comments.
Danny Trebbien wrote on Sun, Feb 20, 2011 at 09:58:07 -0800:
> Attached is version 3 of the patch and log message.
>
>
> >> >> + (! setlocale(LC_ALL, "en_US.ISO-8859-1")) &&
> >> >> + (! setlocale(LC_ALL, "en_GB.ISO-8859-1")) &&
> >> >> + (! setlocale(LC_ALL, "de_DE.ISO-8859-1")))
> >> >> + return svn_error_createf(SVN_ERR_TEST_SKIPPED, NULL, "None of the locales English.1252, German.1252, French.1252, en_US.ISO-8859-1, en_GB.ISO-8859-1, and de_DE.ISO-8859-1 are installed.");
...
> > So, sorry, but I'm not yet convinced that listing all the locales in the
> > log message is a good idea.
>
> Okay. I changed it to "Tried %d locales, but none are installed."
>
Thanks.
> >> +test_svn_subst_translate_string2_null_encoding_helper(apr_pool_t *pool)
> >> +{
> >> + {
> >> + svn_string_t *new_value = NULL;
> >> + svn_boolean_t translated_to_utf8 = FALSE;
> >> + svn_boolean_t translated_line_endings = TRUE;
> >
> > Why did you initialize these two variables? The API will always set
> > them for you, so you shouldn't initialize them.
>
> It's part of the test to check that the API sets them.
>
Fair enough.
> [[[
> Add a test of svn_subst_translate_string2() to the subst_translate test suite.
>
> As discussed at:
> http://thread.gmane.org/gmane.comp.version-control.subversion.devel/125782
> Message-ID: <AANLkTimo=NpF_FqB+PBPkeifXjbnsORGYkZ5npCAxyVz_at_mail.gmail.com>
>
> * subversion/tests/libsvn_subr/subst_translate-test.c
> (test_svn_subst_translate_string2_null_encoding_helper): New function. It is
> the core of the new svn_subst_translate_string2_null_encoding test.
> (test_svn_subst_translate_string2_null_encoding): New test that tests
> svn_subst_translate_string2() with ENCODING set to NULL.
> (test_funcs): Add test_svn_subst_translate_string2_null_encoding().
> ]]]
I tweaked this a bit (e.g., adding ARRAY_LEN() which you neglected to).
> +++ subversion/tests/libsvn_subr/subst_translate-test.c (working copy)
> +/* The body of the svn_subst_translate_string2_null_encoding test. It should
> + only be called by test_svn_subst_translate_string2_null_encoding(), as this
> + code assumes that the process locale has been changed to a locale that uses
> + either CP-1252 or ISO-8859-1 for the default narrow string encoding. */
> static svn_error_t *
> +test_svn_subst_translate_string2_null_encoding_helper(apr_pool_t *pool)
"narrow" string? As opposed to a "wide-character" string?
> +test_svn_subst_translate_string2_null_encoding(apr_pool_t *pool)
> +{
> + char orig_lc_all[256] = { '\0' };
> + svn_error_t *test_result;
...
> + strncpy(orig_lc_all, setlocale(LC_ALL, NULL), sizeof (orig_lc_all));
> +
Typically we use pools (apr_pstrdup()). I haven't changed this.
Received on 2011-02-22 17:14:45 CET