[vlc-devel] [PATCH 20/20] mkv: Reimplemented MkvTree and moved it to `Util.hpp`

[Denis Charmet]

On 2016-03-07 11:07, Filip Roséen wrote:
> Because the whole point of the new code is to get rid of the dynamic
> allocation if that is possible, using a std::string for the same thing
> would require an allocator with static storage - and introducing such
> would in my opinion by far too complex to be worth it.
> 
> The current implementation is cleaner than conditionally using a
> std::string (which indirectly will do dynamic initialization).

In my opinion, adding branching just to potentially avoid dynamic 
allocation is debatable. I didn't run a benchmark but it seems overkill 
in any case. If you are really concerned with the execution speed of a 
non critical function, I would argue that memcpy will be probably be 
faster than strcat :)

> 
> /F
> -------------------------
> 
> On 16/03/07 11:05, Denis Charmet wrote:
> 
>> Hi,
>> 
>> On 2016-03-04 17:04, Filip Roséen wrote:
>> 
>>> +void MkvTree_va( demux_t& demuxer, int i_level, const char* fmt, 
>>> va_list args) +{ + static char const * indent = “|”; + static char 
>>> const * prefix = “+”; + static int const indent_len = strlen( indent 
>>> ); + static int const prefix_len = strlen( prefix ); + + char 
>>> fixed_buffer[256] = {}; + size_t const static_len = sizeof( 
>>> fixed_buffer ); + char * buffer = fixed_buffer; + size_t total_len = 
>>> indent_len * i_level + prefix_len + strlen( fmt ); + + if( total_len 
>>> >= static_len ) { + buffer = new (std::nothrow) char[total_len] (); + 
>>> + if (buffer == NULL) { + msg_Err (&demuxer, “Unable to allocate 
>>> memory for format string”); + return; + } + }
>> 
>> Since you’re cpping everything why not using std::string? :)
>> 
>>> *
>>> * char * dst = buffer;
>>> *
>>> * for (int i = 0; i < i_level; ++i, dst += indent_len)
>>> 
>>> *
>>> 
>>> memcpy( dst, indent, indent_len );
>>> *
>>> * strcat( dst, prefix );
>>> * strcat( dst, fmt );
>>> *
>>> * msg_GenericVa( &demuxer, VLC_MSG_DBG, buffer, args );
>>> *
>>> * if (buffer != fixed_buffer)
>>> 
>>> *
>>> 
>>> delete [] buffer;
>>> +} * +void MkvTree( demux_t & demuxer, int i_level, const char 
>>> *psz_format, … ) +{
>>> * va_list args; va_start( args, psz_format );
>>> * MkvTree_va( demuxer, i_level, psz_format, args );
>>> * va_end( args ); +} diff –git a/modules/demux/mkv/util.hpp 
>>> b/modules/demux/mkv/util.hpp index 8c6cc6a..5867d1d 100644 — 
>>> a/modules/demux/mkv/util.hpp +++ b/modules/demux/mkv/util.hpp @@ 
>>> -90,3 +90,7 @@ public: };
>>> 
>>> block_t * packetize_wavpack( mkv_track_t _, uint8_t _, size_t); + 
>>> +/* helper functions to print the mkv parse tree _/ +void MkvTree_va( 
>>> demux_t& demuxer, int i_level, const char_ fmt, va_list args); +void 
>>> MkvTree( demux_t & demuxer, int i_level, const char *psz_format, … );
>> 
>> REGARDS,
>> 
>> Denis Charmet - TypX Le mauvais esprit est un art de vivre 
>> _______________________________________________ vlc-devel mailing list 
>> To unsubscribe or modify your subscription options: 
>> https://mailman.videolan.org/listinfo/vlc-devel
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel

Regards,
-- 
Denis Charmet - TypX
Le mauvais esprit est un art de vivre