Re: [PATCHv2 0/4] Some cleanups

[Jeff King]

On Wed, Mar 30, 2016 at 10:32:40AM -0700, Stefan Beller wrote:

> > I'm OK with all of these as-is, though I did mention a nit in the third
> > one. I also like Junio's rewrite instead of using strbuf_list_free.
> 
> I'm fine using the rewritten version instead of using strbuf_list_free. :)
> On the third one, there is one case, where we have
> 
>   if (..)
> return error(_(text));
> 
> and that is an exit(128); eventually.

In the caller perhaps, but isn't that equivalent to:

  error(_(text));
  return -1;

?

I think it is OK to make assumptions about error()'s return value; that
is what it is there for.

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

Re: [PATCHv2 0/4] Some cleanups

[Stefan Beller]

On Wed, Mar 30, 2016 at 10:25 AM, Jeff King  wrote:
> On Wed, Mar 30, 2016 at 10:05:14AM -0700, Stefan Beller wrote:
>
>> v2:
>> Thanks Eric, Jeff, Junio for discussion, I picked up all hints and here
>> is a v2.
>>
>> * drop the overallocation patches (1&2)
>> * use git_config_get_string instead of its _const equivalent, such that
>>   we don't need a cast when freeing in git_config_get_notes_strategy
>> * Use strbuf_list_free instead of cooking our own.
>> * have a dedicated error exit path in bundle.c, create_bundle
>
> I'm OK with all of these as-is, though I did mention a nit in the third
> one. I also like Junio's rewrite instead of using strbuf_list_free.

I'm fine using the rewritten version instead of using strbuf_list_free. :)
On the third one, there is one case, where we have

  if (..)
return error(_(text));

and that is an exit(128); eventually.

I thought it is worth preserving the difference (as it is a faithful
bug fix not a
change to make it better or more uniform).

Thanks,
Stefan



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

Re: [PATCHv2 0/4] Some cleanups

[Jeff King]

On Wed, Mar 30, 2016 at 10:05:14AM -0700, Stefan Beller wrote:

> v2:
> Thanks Eric, Jeff, Junio for discussion, I picked up all hints and here
> is a v2.
> 
> * drop the overallocation patches (1&2)
> * use git_config_get_string instead of its _const equivalent, such that
>   we don't need a cast when freeing in git_config_get_notes_strategy
> * Use strbuf_list_free instead of cooking our own.
> * have a dedicated error exit path in bundle.c, create_bundle

I'm OK with all of these as-is, though I did mention a nit in the third
one. I also like Junio's rewrite instead of using strbuf_list_free.

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

[PATCHv2 0/4] Some cleanups

[Stefan Beller]

v2:
Thanks Eric, Jeff, Junio for discussion, I picked up all hints and here
is a v2.

* drop the overallocation patches (1&2)
* use git_config_get_string instead of its _const equivalent, such that
  we don't need a cast when freeing in git_config_get_notes_strategy
* Use strbuf_list_free instead of cooking our own.
* have a dedicated error exit path in bundle.c, create_bundle

v1:
One of my first patches to Git were cleanup patches, and I fell back
to my old pattern here, while thinking on how to write better commit
messages for the submodule bugfixes I currently have in flight.

Just some one liners to not leak memory or file descriptors.

They are bundled as a series, but no patch relies on any predessor.

This applies on v2.8.0.

Thanks,
Stefan

Stefan Beller (4):
  notes: don't leak memory in git_config_get_notes_strategy
  abbrev_sha1_in_line: don't leak memory
  bundle: don't leak an fd in case of early return
  credential-cache, send_request: close fd when done

 builtin/notes.c|  5 +++--
 bundle.c   | 23 +--
 credential-cache.c |  1 +
 wt-status.c|  4 +---
 4 files changed, 22 insertions(+), 11 deletions(-)

-- 
2.8.0.2.gb331331

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