[PATCH] shortlog: initialize nongit variable

Subsystems: the rest

DORMANTno replies

5 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCH] shortlog: initialize nongit variable

From: SZEDER Gábor <hidden>
Date: 2016-06-15 22:44:25

setup_git_directory_gently() only modifies the value of the nongit
variable if we are not in a git repository, therefore nongit is left
uninitialized if we are in a git repository.  This sometimes broke
shortlog when it was not fed with a log on its standard input.

Signed-off-by: SZEDER Gábor <redacted>
---
 builtin-shortlog.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-shortlog.c b/builtin-shortlog.c
index bd795b1..60a6d45 100644
--- a/builtin-shortlog.c
+++ b/builtin-shortlog.c

@@ -229,7 +229,7 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
 {
 	struct shortlog log;
 	struct rev_info rev;
-	int nongit;
+	int nongit = 0;
 
 	prefix = setup_git_directory_gently(&nongit);
 	shortlog_init(&log);

-- 
1.5.5.rc1

Re: [PATCH] shortlog: initialize nongit variable

From: Nguyen Thai Ngoc Duy <hidden>
Date: 2016-06-15 22:44:25

On Tue, Mar 25, 2008 at 9:46 PM, SZEDER Gábor [off-list ref] wrote:

setup_git_directory_gently() only modifies the value of the nongit
 variable if we are not in a git repository, therefore nongit is left
 uninitialized if we are in a git repository.  This sometimes broke
 shortlog when it was not fed with a log on its standard input.

cmd_help() has the same problem. Can you make a similar patch for
cmd_help as well?

Thanks
-- 
Duy

Re: [PATCH] shortlog: initialize nongit variable

From: SZEDER Gábor <hidden>
Date: 2016-06-15 22:44:25

On Tue, Mar 25, 2008 at 10:39:38PM +0700, Nguyen Thai Ngoc Duy wrote:

On Tue, Mar 25, 2008 at 9:46 PM, SZEDER Gábor [off-list ref] wrote:

quoted

setup_git_directory_gently() only modifies the value of the nongit
 variable if we are not in a git repository, therefore nongit is left
 uninitialized if we are in a git repository.  This sometimes broke
 shortlog when it was not fed with a log on its standard input.

cmd_help() has the same problem. Can you make a similar patch for
cmd_help as well?

Yes, I saw that, too, but:

While it's true that cmd_help()'s nongit variable will not be
initialized if we are in a git repository, cmd_help() never touches
nongit after calling setup_git_directory_gently(), so there is no
problem after all.

However, it might be better to let setup_git_directory_gently() set
*nongit_ok to 0 if we are in a git repository.  Patch appended.


Regards,
Gábor



Always set *nongit_ok in setup_git_directory_gently()

setup_git_directory_gently() only modified the value of its *nongit_ok
argument if we were not in a git repository.  Now it will always set it
to 0 when we are inside a repository.

Also remove now unnecessary initializations in the callers of this
function.

Signed-off-by: SZEDER Gábor <redacted>
---
 builtin-bundle.c     |    2 +-
 builtin-config.c     |    2 +-
 builtin-diff-files.c |    2 +-
 builtin-ls-remote.c  |    2 +-
 git.c                |    2 +-
 setup.c              |    8 ++++++++
 6 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/builtin-bundle.c b/builtin-bundle.c
index 9f38e21..ac476e7 100644
--- a/builtin-bundle.c
+++ b/builtin-bundle.c

@@ -14,7 +14,7 @@ static const char *bundle_usage="git-bundle (create <bundle> <git-rev-list args>
 int cmd_bundle(int argc, const char **argv, const char *prefix)
 {
 	struct bundle_header header;
-	int nongit = 0;
+	int nongit;
 	const char *cmd, *bundle_file;
 	int bundle_fd = -1;
 	char buffer[PATH_MAX];

diff --git a/builtin-config.c b/builtin-config.c
index 2b9a426..c34bc8b 100644
--- a/builtin-config.c
+++ b/builtin-config.c

@@ -264,7 +264,7 @@ static int get_colorbool(int argc, const char **argv)
 
 int cmd_config(int argc, const char **argv, const char *prefix)
 {
-	int nongit = 0;
+	int nongit;
 	char* value;
 	const char *file = setup_git_directory_gently(&nongit);

diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index 4abe3c2..e2306c1 100644
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c

@@ -16,7 +16,7 @@ COMMON_DIFF_OPTIONS_HELP;
 int cmd_diff_files(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info rev;
-	int nongit = 0;
+	int nongit;
 	int result;
 
 	prefix = setup_git_directory_gently(&nongit);

diff --git a/builtin-ls-remote.c b/builtin-ls-remote.c
index 8907a89..06ab8da 100644
--- a/builtin-ls-remote.c
+++ b/builtin-ls-remote.c

@@ -31,7 +31,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 {
 	int i;
 	const char *dest = NULL;
-	int nongit = 0;
+	int nongit;
 	unsigned flags = 0;
 	const char *uploadpack = NULL;
 	const char **pattern = NULL;

diff --git a/git.c b/git.c
index 13de801..70458af 100644
--- a/git.c
+++ b/git.c

@@ -142,7 +142,7 @@ static int split_cmdline(char *cmdline, const char ***argv)
 
 static int handle_alias(int *argcp, const char ***argv)
 {
-	int nongit = 0, envchanged = 0, ret = 0, saved_errno = errno;
+	int nongit, envchanged = 0, ret = 0, saved_errno = errno;
 	const char *subdir;
 	int count, option_count;
 	const char** new_argv;

diff --git a/setup.c b/setup.c
index 41e298b..3d2d958 100644
--- a/setup.c
+++ b/setup.c

@@ -326,6 +326,14 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	int len, offset;
 
 	/*
+	 * Let's assume that we are in a git repository.
+	 * If it turns out later that we are somewhere else, the value will be
+	 * updated accordingly.
+	 */
+	if (nongit_ok)
+		*nongit_ok = 0;
+
+	/*
 	 * If GIT_DIR is set explicitly, we're not going
 	 * to do any discovery, but we still do repository
 	 * validation.

-- 
1.5.5.rc1

Re: [PATCH] shortlog: initialize nongit variable

From: Jonas Fonseca <hidden>
Date: 2016-06-15 22:44:25

2008/3/25 SZEDER Gábor [off-list ref]:

 However, it might be better to let setup_git_directory_gently() set
 *nongit_ok to 0 if we are in a git repository.  Patch appended.

As the one responsible for introducing this bug into shortlog, I can only agree.

  builtin-bundle.c     |    2 +-
  builtin-config.c     |    2 +-
  builtin-diff-files.c |    2 +-
  builtin-ls-remote.c  |    2 +-
  git.c                |    2 +-
  setup.c              |    8 ++++++++
  6 files changed, 13 insertions(+), 5 deletions(-)

git grep reveals that builtin-{apply,diff}.c, help.c and var.c also
belongs on this list.

-- 
Jonas Fonseca

Re: [PATCH] shortlog: initialize nongit variable

From: SZEDER Gábor <hidden>
Date: 2016-06-15 22:44:25

On Tue, Mar 25, 2008 at 08:58:50PM +0100, Jonas Fonseca wrote:

2008/3/25 SZEDER Gábor [off-list ref]:

quoted

  builtin-bundle.c     |    2 +-
  builtin-config.c     |    2 +-
  builtin-diff-files.c |    2 +-
  builtin-ls-remote.c  |    2 +-
  git.c                |    2 +-
  setup.c              |    8 ++++++++
  6 files changed, 13 insertions(+), 5 deletions(-)

git grep reveals that builtin-{apply,diff}.c,

Right, I have overseen them.  Updated patch appended.

help.c and var.c also belongs on this list.

They don't initialize (and use) their nongit variable.

Regards,
Gábor


Always set *nongit_ok in setup_git_directory_gently()

setup_git_directory_gently() only modified the value of its *nongit_ok
argument if we were not in a git repository.  Now it will always set it
to 0 when we are inside a repository.

Also remove now unnecessary initializations in the callers of this
function.

Signed-off-by: SZEDER Gábor <redacted>
---
 builtin-apply.c      |    2 +-
 builtin-bundle.c     |    2 +-
 builtin-config.c     |    2 +-
 builtin-diff-files.c |    2 +-
 builtin-diff.c       |    2 +-
 builtin-ls-remote.c  |    2 +-
 git.c                |    2 +-
 setup.c              |    8 ++++++++
 8 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index a3f075d..b5f78ac 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c

@@ -2994,7 +2994,7 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
 	int read_stdin = 1;
 	int inaccurate_eof = 0;
 	int errs = 0;
-	int is_not_gitdir = 0;
+	int is_not_gitdir;
 
 	const char *whitespace_option = NULL;

diff --git a/builtin-bundle.c b/builtin-bundle.c
index 9f38e21..ac476e7 100644
--- a/builtin-bundle.c
+++ b/builtin-bundle.c

@@ -14,7 +14,7 @@ static const char *bundle_usage="git-bundle (create <bundle> <git-rev-list args>
 int cmd_bundle(int argc, const char **argv, const char *prefix)
 {
 	struct bundle_header header;
-	int nongit = 0;
+	int nongit;
 	const char *cmd, *bundle_file;
 	int bundle_fd = -1;
 	char buffer[PATH_MAX];

diff --git a/builtin-config.c b/builtin-config.c
index 2b9a426..c34bc8b 100644
--- a/builtin-config.c
+++ b/builtin-config.c

@@ -264,7 +264,7 @@ static int get_colorbool(int argc, const char **argv)
 
 int cmd_config(int argc, const char **argv, const char *prefix)
 {
-	int nongit = 0;
+	int nongit;
 	char* value;
 	const char *file = setup_git_directory_gently(&nongit);

diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index 4abe3c2..e2306c1 100644
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c

@@ -16,7 +16,7 @@ COMMON_DIFF_OPTIONS_HELP;
 int cmd_diff_files(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info rev;
-	int nongit = 0;
+	int nongit;
 	int result;
 
 	prefix = setup_git_directory_gently(&nongit);

diff --git a/builtin-diff.c b/builtin-diff.c
index 444ff2f..7c2a841 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c

@@ -210,7 +210,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	int ents = 0, blobs = 0, paths = 0;
 	const char *path = NULL;
 	struct blobinfo blob[2];
-	int nongit = 0;
+	int nongit;
 	int result = 0;
 
 	/*

diff --git a/builtin-ls-remote.c b/builtin-ls-remote.c
index 8907a89..06ab8da 100644
--- a/builtin-ls-remote.c
+++ b/builtin-ls-remote.c

@@ -31,7 +31,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 {
 	int i;
 	const char *dest = NULL;
-	int nongit = 0;
+	int nongit;
 	unsigned flags = 0;
 	const char *uploadpack = NULL;
 	const char **pattern = NULL;

diff --git a/git.c b/git.c
index 13de801..70458af 100644
--- a/git.c
+++ b/git.c

@@ -142,7 +142,7 @@ static int split_cmdline(char *cmdline, const char ***argv)
 
 static int handle_alias(int *argcp, const char ***argv)
 {
-	int nongit = 0, envchanged = 0, ret = 0, saved_errno = errno;
+	int nongit, envchanged = 0, ret = 0, saved_errno = errno;
 	const char *subdir;
 	int count, option_count;
 	const char** new_argv;

diff --git a/setup.c b/setup.c
index 41e298b..3d2d958 100644
--- a/setup.c
+++ b/setup.c

@@ -326,6 +326,14 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	int len, offset;
 
 	/*
+	 * Let's assume that we are in a git repository.
+	 * If it turns out later that we are somewhere else, the value will be
+	 * updated accordingly.
+	 */
+	if (nongit_ok)
+		*nongit_ok = 0;
+
+	/*
 	 * If GIT_DIR is set explicitly, we're not going
 	 * to do any discovery, but we still do repository
 	 * validation.

-- 
1.5.5.rc1.7.g249d1

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help