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+pbpkeifxjbnsorgykz5npcax...@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.