Re: [PATCH] Fix to svn_time_to_human_cstring()

Nicolás Lichtmaier <nick@reloco.com.ar> writes:

> * subversion/libsvn_subr/time.c:
> (svn_time_to_human_cstring) Despite its name (cstring) it is needed
> that this function return an UTF8 string. So there's the need to
> convert

Subversion converts both ways, i.e., both time to string and string to
time. Why does only one of these do UTF-8 conversion?

> the human-readable part, which can come encoded in the current
> locale's charset.
>
> Index: subversion/libsvn_subr/time.c
> ===================================================================
> --- subversion/libsvn_subr/time.c (revision 9284)
> +++ subversion/libsvn_subr/time.c (working copy)
> @@ -226,7 +226,7 @@
> apr_time_exp_t exploded_time;
> apr_size_t len, retlen;
> apr_status_t ret;
> - char *datestr, *curptr;
> + char *datestr, *curptr, human_datestr[SVN_TIME__MAX_LENGTH];
>
> /* Get the time into parts */
> apr_time_exp_lt (&exploded_time, when);
> @@ -255,7 +255,7 @@
> curptr = datestr + len;
>
> /* Put in human explanatory part */
> - ret = apr_strftime (curptr,
> + ret = apr_strftime (human_datestr,
> &retlen,
> SVN_TIME__MAX_LENGTH - len,
> human_timestamp_format_suffix,
> @@ -264,7 +264,22 @@
> /* If there was an error, ensure that the string is zero-terminated. */
> if (ret || retlen == 0)
> *curptr = '\0';
> + else
> + {
> + char *utf8_string;
> + svn_error_t *err;
>
> + err = svn_utf_cstring_to_utf8(&utf8_string, human_datestr, pool);
> + if(err)
> + {
> + *curptr = '\0';

That's a big problem, it's leaking an error. Is it acceptable for
this function to clear and ignore an error? Perhaps we need a new
signature that returns an svn_error_t*?

> + } else

Put else on a separate line please.

> + {
> + strncpy(curptr, utf8_string, SVN_TIME__MAX_LENGTH - len);
> + curptr[SVN_TIME__MAX_LENGTH - len] = '\0';

apr_cpystrn would be neater.

> + }
> + }
> +
> return datestr;

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org