Re: [PATCH 06/34] clone: release strbuf after use in remove_junk()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Rene Scharfe <l.s.r@xxxxxx> writes:

> Signed-off-by: Rene Scharfe <l.s.r@xxxxxx>
> ---
>  builtin/clone.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 8d11b570a1..dbddd98f80 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -487,28 +487,28 @@ N_("Clone succeeded, but checkout failed.\n"
> ...
>  	if (junk_git_dir) {
>  		strbuf_addstr(&sb, junk_git_dir);
>  		remove_dir_recursively(&sb, 0);
>  		strbuf_reset(&sb);
>  	}
>  	if (junk_work_tree) {
>  		strbuf_addstr(&sb, junk_work_tree);
>  		remove_dir_recursively(&sb, 0);
> -		strbuf_reset(&sb);
>  	}
> +	strbuf_release(&sb);
>  }

The code definitely needs a _release() at the end, but I feel
lukewarm about the "if we are about to _release(), do not bother to
_reset()" micro-optimization.  Keeping the existing two users that
use sb as a (shared and reused) temporary similar would help those
who add the third one or reuse the pattern in their code elsewhere.

We could flip the "be nice to the next user by clearing after use"
pattern to "clear any potential cruft before you use", i.e.

	if (...) {
		strbuf_reset(&sb);
		strbuf_addstr(&sb, ...);
	}
	if (...) {
		strbuf_reset(&sb);
		strbuf_addstr(&sb, ...);
	}
	strbuf_release(&sb);

but that still tempts us for the same micro-optimization at the
beginning where sb hasn't been used since STRBUF_INIT, so it is not
a real "solution".

So, I dunno.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux