All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Alexander Kuleshov <kuleshovmail@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] cat-file: Move assignment to the buffer declaration
Date: Thu, 08 Jan 2015 11:31:46 -0800	[thread overview]
Message-ID: <xmqq61chf4rh.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1420741203-21107-1-git-send-email-kuleshovmail@gmail.com> (Alexander Kuleshov's message of "Fri, 9 Jan 2015 00:20:03 +0600")

Alexander Kuleshov <kuleshovmail@gmail.com> writes:

> Subject: Re: [PATCH] cat-file: Move assignment to the buffer declaration

"git shortlog" on recent history shows that it is conventional not
to upcase the sentence after the "<area>:" on titles.

This line not just "declares" but "defines" buf, so I'd phrase it
more like:

	cat-file: initialize buf to NULL at its definition

	This way is shorter than with a separate assignment.

or somesuch.

If two people posted two patches to add a new cat-file.c file and
one patch did it the way the current code does while the other patch
did it the way the code with this patch does, I would likely to pick
the latter, because it is more concise and it is more clear that
there is less chance that buf could be used uninitialized.

To be bluntly honest, however, it's such a minor thing that once it
_is_ in the tree, it's not really worth the patch noise to go and
fix it up.

> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
>  builtin/cat-file.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index f8d8129..840ace2 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -17,14 +17,13 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
>  {
>  	unsigned char sha1[20];
>  	enum object_type type;
> -	char *buf;
> +	char *buf = NULL;
>  	unsigned long size;
>  	struct object_context obj_context;
>  
>  	if (get_sha1_with_context(obj_name, 0, sha1, &obj_context))
>  		die("Not a valid object name %s", obj_name);
>  
> -	buf = NULL;



>  	switch (opt) {
>  	case 't':
>  		type = sha1_object_info(sha1, NULL);

      reply	other threads:[~2015-01-08 19:32 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-08 18:20 [PATCH] cat-file: Move assignment to the buffer declaration Alexander Kuleshov
2015-01-08 19:31 ` Junio C Hamano [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq61chf4rh.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=kuleshovmail@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.