Re: Add svn_revnum_to_cstring function

[lu...@softhome.net]

Erik Huelsmann writes: 

>  
> 
> 
> I quickly read your patch below and have put some comments there. 
> 
> 
Thanks!
> 
>>  (svn_revnum_to_cstring): New function.
>>* subversion/libsvn_subr/revnum.c
>>  New file.
> Possibly there are other type conversions necessary (although not yet
> identified) for SVN or APR types. Those could be added to this new file if
> it were named type-convertion.c (or something alike). 
> 
What about type_conversion.c? 

>>+{
>>+  svn_error_t *err;
>>+  const char *s;
>>+  const char *ret;
>>+
>>+  s = apr_psprintf (pool, "%" SVN_REVNUM_T_FMT, revnum);
>>+
>>+  err = svn_utf_cstring_to_utf8 (&ret, s, pool);
>>+
>>+  if (err) {
>>+    svn_error_clear (err);
>>+    /* Better to give out something than nothing */
> 
> Indeed, but not something in the wrong encoding... Hmmm; maybe it just
> became impossible to call this conversion in-line because of the need to be
> able to error out... ? grep seems to indicate a rewrite of 'only' 178
> instances. I need some time to think about a proper solution. 
> 
Do we really need to be that paranoid? We know that the revision number is a 
decimal number (but it might be a long, int or long long or something). But 
it should possibly only contain the digits 0-9 (and a minus if it is 
invalid). So instead of having to support returning an svn_error_t * in 
cases that shouldn't happen in practice, couldn't we just hardcode the UTF-8 
conversion for this special case? this menas converting '0' to 48, '1' to 49 
etc. The UTF-8 coding of ASCII characters is set in stone and apr_psprintf 
uses the execution character set. This might sound ugly, but isn't it better 
to do this than to complicate all 200 places where the function is used? 

//Peter 


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