Skip to contentSkip to conversation

submodule-parallel-fetch: make some file local symbols static

PATCH In review Ramsay Jones 3 participants 4 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 submodule-parallel-fetch: make some file local symbols static

Commits 0fc1fdb0 ("fetch_populated_submodules: use new parallel job processing", 28-09-2015) and 60f24f52 ("run-command: add an asynchronous parallel child processor", 28-09-2015) both introduce external symbols which only require file scope visibility. In order to reduce the visibility, apply the static keyword to their declarations.

---

Hi Stefan,

No, despite the same subject, this is not the same patch that I sent you last week! :-D

Could you please squash parts of this into the patches corresponding to the above mentioned commits.

Thanks!

BTW, I would once again suggest that you could move the definition of get_next_submodule() to be above/before fetch_populated_submodules() so that you can remove the forward declaration.

ATB,
Ramsay Jones

 run-command.c | 2 +-
 submodule.c   | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)
run-command.c +1 -1
index 341b23b..347d22e 100644
@@ -865,7 +865,7 @@ int capture_command(struct child_process *cmd, struct strbuf *buf, size_t hint)
 	return finish_command(cmd);
 }
 
-struct parallel_processes {
+static struct parallel_processes {
 	void *data;
 
 	int max_processes;
submodule.c +4 -4
index bd6e208..638efb5 100644
@@ -622,8 +622,8 @@ struct submodule_parallel_fetch {
 };
 #define SPF_INIT {0, ARGV_ARRAY_INIT, NULL, NULL, 0, 0, 0}
 
-int get_next_submodule(void *data, struct child_process *cp,
-		       struct strbuf *err);
+static int get_next_submodule(void *data, struct child_process *cp,
+			      struct strbuf *err);
 
 static int fetch_start_failure(void *data, struct child_process *cp,
 			       struct strbuf *err)
@@ -682,8 +682,8 @@ out:
 	return spf.result;
 }
 
-int get_next_submodule(void *data, struct child_process *cp,
-		       struct strbuf *err)
+static int get_next_submodule(void *data, struct child_process *cp,
+			      struct strbuf *err)
 {
 	int ret = 0;
 	struct submodule_parallel_fetch *spf = data;
Signature · 1 line
2.6.0
#1 lore raw
Quoted 10 lines · Ramsay Jones

Commits 0fc1fdb0 ("fetch_populated_submodules: use new parallel job processing", 28-09-2015) and 60f24f52 ("run-command: add an asynchronous parallel child processor", 28-09-2015) both introduce external symbols which only require file scope visibility. In order to reduce the visibility, apply the static keyword to their declarations.

---

Quoted 7 lines

Hi Stefan,

No, despite the same subject, this is not the same patch that I sent you last week! :-D

Could you please squash parts of this into the patches corresponding to the above mentioned commits.

I am sorry for the need to send this second patch. :(

Quoted 19 lines

Thanks!

BTW, I would once again suggest that you could move the definition of get_next_submodule() to be above/before fetch_populated_submodules() so that you can remove the forward declaration.

ATB,
Ramsay Jones

 run-command.c | 2 +-
 submodule.c   | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)
run-command.c +0 -0
index 341b23b..347d22e 100644

run-command.c int capture_command(struct child_process *cmd, struct strbuf *buf, size_t hint) · line 868

Quoted 5 lines
run-command.c +1 -1
@@ -865,7 +865,7 @@ int capture_command(struct child_process *cmd, struct strbuf *buf, size_t hint)
        return finish_command(cmd);
 }
 
-struct parallel_processes {
+static struct parallel_processes {

will pickup in a reroll

Quoted 8 lines
run-command.c +0 -0
        void *data;
 
        int max_processes;
submodule.c +0 -0
index bd6e208..638efb5 100644

submodule.c struct submodule_parallel_fetch { · line 626

Quoted 7 lines
submodule.c +2 -2
@@ -622,8 +622,8 @@ struct submodule_parallel_fetch {
 };
 #define SPF_INIT {0, ARGV_ARRAY_INIT, NULL, NULL, 0, 0, 0}
 
-int get_next_submodule(void *data, struct child_process *cp,
-                      struct strbuf *err);
+static int get_next_submodule(void *data, struct child_process *cp,
+                             struct strbuf *err);

I thought I had this in yesterdays reroll (v6). Oh you're referring to the version
from the 28th (I forgot to label them v5 I suppose).

I will also get rid of the forward declaration.

Quoted 20 lines
submodule.c +2 -3
 static int fetch_start_failure(void *data, struct child_process *cp,
                               struct strbuf *err)
@@ -682,8 +682,8 @@ out:
        return spf.result;
 }
 
-int get_next_submodule(void *data, struct child_process *cp,
-                      struct strbuf *err)
+static int get_next_submodule(void *data, struct child_process *cp,
+                             struct strbuf *err)
 {
        int ret = 0;
        struct submodule_parallel_fetch *spf = data;
--

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

#2 lore raw
Quoted 3 lines · Stefan Beller
Quoted 1 line · Ramsay Jones

[snip]

Quoted 8 lines
Quoted 8 lines
run-command.c +0 -0
index 341b23b..347d22e 100644
@@ -865,7 +865,7 @@ int capture_command(struct child_process *cmd, struct strbuf *buf, size_t hint)
        return finish_command(cmd);
 }
 
Quoted 4 lines

run-command.c int capture_command(struct child_process *cmd, struct strbuf *buf, size_t hint) · line 868

Quoted 2 lines
run-command.c +1 -1
-struct parallel_processes {
+static struct parallel_processes {

will pickup in a reroll

Thanks

Quoted 12 lines
Quoted 12 lines
run-command.c +0 -0
        void *data;
 
        int max_processes;
submodule.c +0 -0
index bd6e208..638efb5 100644
@@ -622,8 +622,8 @@ struct submodule_parallel_fetch {
 };
 #define SPF_INIT {0, ARGV_ARRAY_INIT, NULL, NULL, 0, 0, 0}
 
Quoted 8 lines

submodule.c struct submodule_parallel_fetch { · line 626

Quoted 4 lines
submodule.c +2 -2
-int get_next_submodule(void *data, struct child_process *cp,
-                      struct strbuf *err);
+static int get_next_submodule(void *data, struct child_process *cp,
+                             struct strbuf *err);

I thought I had this in yesterdays reroll (v6). Oh you're referring to the version
from the 28th (I forgot to label them v5 I suppose).

Ah! I thought I'd seen it on the list. (I thought I was going crazy) ;-) Sorry, my fault. I just assumed that today's pu branch would have your latest patches - I didn't actually check that.

Note that the first hunk, above, is actually new (I hadn't noticed it before).

Quoted 2 lines

I will also get rid of the forward declaration.

Thanks!

ATB,
Ramsay Jones

Quoted 16 lines
Quoted 16 lines
submodule.c +2 -3
 static int fetch_start_failure(void *data, struct child_process *cp,
                               struct strbuf *err)
@@ -682,8 +682,8 @@ out:
        return spf.result;
 }
 
-int get_next_submodule(void *data, struct child_process *cp,
-                      struct strbuf *err)
+static int get_next_submodule(void *data, struct child_process *cp,
+                             struct strbuf *err)
 {
        int ret = 0;
        struct submodule_parallel_fetch *spf = data;
--
#3 lore raw
Quoted 7 lines · Ramsay Jones
Quoted 3 lines

I thought I had this in yesterdays reroll (v6). Oh you're referring to the version
from the 28th (I forgot to label them v5 I suppose).

Ah! I thought I'd seen it on the list. (I thought I was going crazy) ;-) Sorry, my fault. I just assumed that today's pu branch would have your latest patches - I didn't actually check that.

Sometimes I take a one-day vacation ;-)

Thanks.