Re: [PATCH 2/2] log: do not shorten decoration names too early

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

 



On Thu, May 14, 2015 at 03:25:33PM -0700, Junio C Hamano wrote:

> @@ -90,6 +97,8 @@ static MAYBE_UNUSED elemtype *slabname## _at(struct slabname *s,	\
>  									\
>  	if (s->slab_count <= nth_slab) {				\
>  		int i;							\
> +		if (!add_if_missing)					\
> +			return NULL;					\
>  		s->slab = xrealloc(s->slab,				\
>  				   (nth_slab + 1) * sizeof(*s->slab));	\
>  		stat_ ##slabname## realloc++;				\

This skips extending the list of slabs if we would go past the nth slab.
But we don't fill in each slab in the first place. I.e., we may have 10
slabs, but only s->slab[10] is non-NULL.

A few lines below this, we xcalloc() it if necessary. I think that needs
to respect add_if_missing, as well.

>  void unuse_commit_buffer(const struct commit *commit, const void *buffer)
>  {
> -	struct commit_buffer *v = buffer_slab_at(&buffer_slab, commit);
> -	if (v->buffer != buffer)
> +	struct commit_buffer *v = buffer_slab_peek(&buffer_slab, commit);
> +	if (v && v->buffer != buffer)
>  		free((void *)buffer);
>  }

I think you want:

  if (!v || v->buffer != buffer)

here. IOW, we free it only if it is not our cached buffer, and it cannot
be if we do not have a cached buffer. It may be easier to read by
flipping the logic:

  if (v && v->buffer == buffer)
	return; /* it is saved in the cache */
  free((void *)buffer);

Or some variation on that.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[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]