Re: [Bug-tar] [PATCH] tar: simplify code in system.c

[Pavel Raiskup]

On Wed, 2013-02-20 at 16:28 +0300, Sergey Poznyakoff wrote:
> Pavel Raiskup <address@hidden> ha escrit:
> 
> > Move the xexec function into #ifndef MSDOS part of file.
> 
> Yes, that's reasonable.
> 
> > * system.c (try_exec): New function - modified previous xexec.
> 
> This will cause a memory leak and imply unnecessary memory
> reallocations.
> 
> What happens here is:
> 
> 487      if (wordsplit (p, &ws, wsflags))
> 488        FATAL_ERROR ((0, 0, _("cannot split string '%s': %s"),
> 489                      p, wordsplit_strerror (&ws)));
> 490      wsflags |= WRDSF_REUSE;
> 
> after the first call to wordsplit the WRDSF_REUSE flag is set, which
> forces wordsplit to reuse already allocated memory on the next iteration
> of the loop.
>
> If using the proposed try_exec, then (1) the memory allocated by each
> call to wordsplit is never freed, (2) an extra call to xmalloc causes
> unnecessary memory allocation, which is never freed as well, and (3)
> each subsequent call to wordsplit allocates new blocks of memory instead
> of reusing the previously allocated ones. Besides, shifting a memory
> array and adding an extra element is much faster than allocating a new
> block of memory for the sole purpose of tucking an extra "-d" in it.

Yes, thats true.  We are not worry about memory leaks (even when it is
solvable) in that case but the performance point of view is really
important.  I may look at the de-duplication problem more carefully if you
want.. but it is so important now.

Pavel