B. Smith-Mannschott wrote:
> On Sun, Apr 19, 2009 at 22:28, Stefan Sperling <stsp_at_elego.de> wrote:
>
>> On Sun, Apr 19, 2009 at 07:55:26PM +0200, B. Smith-Mannschott wrote:
>>
>>> I've been reworking my patch for #3404 and the first piece is ready
>>> for review, so I though I'd post it.
>>>
>>> [[[
>>> Issue #3404: Provide string function to normalize eol-style.
>>>
>>> * subversion/subversion/include/svn_string.h
>>> (svn_string_with_normalized_eol_style): new function
>>> * subversion/libsvn_subr/svn_string.c
>>> (svn_string_with_normalized_eol_style): new function
>>> ]]]
>>>
>> How is this different from svn_subst_translate_cstring2() ?
>>
>> Quoting svn_subst.h:
>>
>> /**
>> * Convenience routine: a variant of svn_subst_translate_stream3() which
>> * operates on cstrings.
>> *
>> * @since New in 1.3.
>> *
>> * Return a new string in @a *dst, allocated in @a pool, by copying the
>> * contents of string @a src, possibly performing line ending and keyword
>> * translations.
>> *
>> * If @a eol_str and @a keywords are @c NULL, behavior is just a byte-for-byte
>> * copy.
>> */
>> svn_error_t *
>> svn_subst_translate_cstring2(const char *src,
>> const char **dst,
>> const char *eol_str,
>> svn_boolean_t repair,
>> apr_hash_t *keywords,
>> svn_boolean_t expand,
>> apr_pool_t *pool);
>>
>>
>
> I was not aware of svn_subst_translate_cstring2(), just as I'm not
> aware of -- oh, I'd say 95% of the functions that make up Subversion.
> The code is still *very* new to me.
>
> So, now I've looked it up, and followed it right down the rabbit hole.
> I've read it, skimmed the things it calls and tried to reason my way
> through the indirection via function pointers...
>
> * svn_subst_translate_cstring2 37
> *** svn_stream_from_stringbuf 18
> ***** svn_stream_create 12
> ***** (read_handler_stringbuf)
> ***** (write_handler_stringbuf)
> *** svn_stream_write 6
> ***** -> write_handler_stringbuf 8
> *** svn_subst_stream_translated 62
> ***** svn_stream_create 12
> ***** create_translation_baton 25
> ***** (translated_stream_read) 52
> ****** translate_chunk 161
> ******** translate_newline 32
> *********** translate_write 12
> ***** (translated_stream_write) 11
> ***** (translated_stream_close) 13
> ----------------------------------
> 463
>
> Finally, in translate_chunk I found the logic that identifies new
> lines. It's rather hard to follow because of course It's doing keyword
> substitution at the same time and normalizing to an arbitrary
> eol-style, not just LF. Oh, and it'll optionally error out for you
> when it discovers inconsistencies, but I don't need that. In the end,
> I think it behaves as my function is intended to. I could use it, I
> suppose. Perhaps I should. (code reuse...)
>
> Still, it seems like overkill to get about 500 lines of svn_stream and
> translation machinery involved just to achieve eol-normalization of a
> string, which is already entirely in memory.
>
I tend to sort of agree here ... I've been amazed by how opaque our
stream translation code is before. Perhaps if you abstract the
EOL-normalization out of translate_chunk and put it as a separate
function in svn_string?
The point of the whole exercise being, of course, that it does not
suddenly require twice as many allocations in the stream-translation
call stack.
-- Brane
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1847389
Received on 2009-04-21 21:11:14 CEST