* [PATCH 1/3] sparse: Fix some "symbol not declared" warnings
@ 2011-09-11 19:38 Ramsay Jones
2011-09-11 21:38 ` Junio C Hamano
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Ramsay Jones @ 2011-09-11 19:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list
In particular, sparse issues the "symbol 'a_symbol' was not declared.
Should it be static?" warning for the following symbols:
submodule.c:321:5: 'submodule_needs_pushing'
submodule.c:355:5: 'push_submodule'
builtin/revert.c:662:20: 'commit_list_append'
These symbols only require file scope, so we simply add the static
modifier to their declarations.
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
builtin/revert.c | 4 ++--
submodule.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 5e97622..5f1cee8 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -659,8 +659,8 @@ static void read_and_refresh_cache(struct replay_opts *opts)
* assert(commit_list_count(list) == 2);
* return list;
*/
-struct commit_list **commit_list_append(struct commit *commit,
- struct commit_list **next)
+static struct commit_list **commit_list_append(struct commit *commit,
+ struct commit_list **next)
{
struct commit_list *new = xmalloc(sizeof(struct commit_list));
new->item = commit;
diff --git a/submodule.c b/submodule.c
index 38d9877..5a02890 100644
--- a/submodule.c
+++ b/submodule.c
@@ -318,7 +318,7 @@ static int has_remote(const char *refname, const unsigned char *sha1, int flags,
return 1;
}
-int submodule_needs_pushing(const char *path, const unsigned char sha1[20], void *data)
+static int submodule_needs_pushing(const char *path, const unsigned char sha1[20], void *data)
{
int *needs_pushing = data;
@@ -352,7 +352,7 @@ int submodule_needs_pushing(const char *path, const unsigned char sha1[20], void
return 1;
}
-int push_submodule(const char *path, const unsigned char sha1[20], void *data)
+static int push_submodule(const char *path, const unsigned char sha1[20], void *data)
{
if (add_submodule_odb(path) || !lookup_commit_reference(sha1))
return 1;
--
1.7.6
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 1/3] sparse: Fix some "symbol not declared" warnings 2011-09-11 19:38 [PATCH 1/3] sparse: Fix some "symbol not declared" warnings Ramsay Jones @ 2011-09-11 21:38 ` Junio C Hamano 2011-09-12 0:06 ` Junio C Hamano 2011-09-11 21:38 ` [PATCH 1/3] make-static: master Junio C Hamano ` (2 subsequent siblings) 3 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2011-09-11 21:38 UTC (permalink / raw) To: Ramsay Jones; +Cc: GIT Mailing-list There are many others that can be identified by running the attached script after you build your git. I'll follow this message up with three consolidated patches, but the latter two needs to be split into smaller pieces to be either queued on top (for topics in next) or squashed into (for topics in pu), which I am not going to do myself today (hint, hint). #!/usr/bin/perl -w my %defd = (); my %used = (); my %def_ok = map { $_ => 1 } qw( main alloc_report have_git_dir prepare_git_cmd print_string_list tm_to_time_t unsorted_string_list_has_string xdl_atol xdl_cha_first xdl_cha_next xdl_mmfile_size xdl_num_out xdl_recs_cmp ); for (<*.o>, <*/*.o>, <*/*/*.o>) { my $obj = $_; open(I, "-|", qw(nm -g), $obj) or die; while (<I>) { unless (/^[0-9a-f ]+([A-Z]) (\S*)$/) { print STDERR "? $_"; next; } if (($1 eq "U") || $1 eq "C") { $used{$2}++; } else { push @{$defd{$obj}}, $2; } } close I; } for my $obj (sort keys %defd) { my $syms = $defd{$obj}; for my $sym (@$syms) { next if exists $used{$sym} or exists $def_ok{$sym}; print "$obj - $sym\n"; } } ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] sparse: Fix some "symbol not declared" warnings 2011-09-11 21:38 ` Junio C Hamano @ 2011-09-12 0:06 ` Junio C Hamano 0 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2011-09-12 0:06 UTC (permalink / raw) To: GIT Mailing-list; +Cc: Ramsay Jones Junio C Hamano <gitster@pobox.com> writes: > There are many others that can be identified by running the attached > script after you build your git. I'll follow this message up with three > consolidated patches, but the latter two needs to be split into smaller > pieces to be either queued on top (for topics in next) or squashed into > (for topics in pu), which I am not going to do myself today (hint, hint). Well, I lied ;-) I've separated things out, queued fix-ups at the tips of branches, and pushed out the integration result to the public repositories (other than k.org that seems to be still down). ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] make-static: master 2011-09-11 19:38 [PATCH 1/3] sparse: Fix some "symbol not declared" warnings Ramsay Jones 2011-09-11 21:38 ` Junio C Hamano @ 2011-09-11 21:38 ` Junio C Hamano 2011-09-12 6:14 ` Ramkumar Ramachandra ` (2 more replies) 2011-09-11 21:38 ` [PATCH 2/3] make-static: next Junio C Hamano 2011-09-11 21:38 ` [PATCH 3/3] make-static: pu Junio C Hamano 3 siblings, 3 replies; 14+ messages in thread From: Junio C Hamano @ 2011-09-11 21:38 UTC (permalink / raw) To: Ramsay Jones; +Cc: GIT Mailing-list Many symbols that are exported to the global scope do not have to be. Signed-off-by: Junio C Hamano <junio@pobox.com> --- * To be applied on top of 3793ac5 (RelNotes/1.7.7: minor fixes, 2011-09-07) builtin.h | 2 -- builtin/notes.c | 4 ++-- cache.h | 2 -- convert.c | 2 +- convert.h | 1 - diff.c | 2 +- diff.h | 1 - dir.c | 4 ++-- dir.h | 2 -- git-compat-util.h | 5 ----- graph.c | 28 ++++++++++++++++++++++++++-- graph.h | 29 ----------------------------- notes.c | 15 +++++++++++++-- notes.h | 14 -------------- parse-options.c | 2 +- parse-options.h | 1 - rerere.c | 2 +- rerere.h | 1 - sha1-array.c | 2 +- sha1-array.h | 1 - trace.c | 2 +- wrapper.c | 40 ++++++++++++++++++++-------------------- 22 files changed, 69 insertions(+), 93 deletions(-) diff --git a/builtin.h b/builtin.h index 0e9da90..8f50a3a 100644 --- a/builtin.h +++ b/builtin.h @@ -16,7 +16,6 @@ extern const char git_more_info_string[]; extern void prune_packed_objects(int); extern int fmt_merge_msg(struct strbuf *in, struct strbuf *out, int merge_title, int shortlog_len); -extern void commit_notes(struct notes_tree *t, const char *msg); struct notes_rewrite_cfg { struct notes_tree **trees; @@ -28,7 +27,6 @@ struct notes_rewrite_cfg { int mode_from_env; }; -combine_notes_fn parse_combine_notes_fn(const char *v); struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd); int copy_note_for_rewrite(struct notes_rewrite_cfg *c, const unsigned char *from_obj, const unsigned char *to_obj); diff --git a/builtin/notes.c b/builtin/notes.c index f8e437d..34b74ee 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -288,7 +288,7 @@ static int parse_reedit_arg(const struct option *opt, const char *arg, int unset return parse_reuse_arg(opt, arg, unset); } -void commit_notes(struct notes_tree *t, const char *msg) +static void commit_notes(struct notes_tree *t, const char *msg) { struct strbuf buf = STRBUF_INIT; unsigned char commit_sha1[20]; @@ -312,7 +312,7 @@ void commit_notes(struct notes_tree *t, const char *msg) strbuf_release(&buf); } -combine_notes_fn parse_combine_notes_fn(const char *v) +static combine_notes_fn parse_combine_notes_fn(const char *v) { if (!strcasecmp(v, "overwrite")) return combine_notes_overwrite; diff --git a/cache.h b/cache.h index 607c2ea..e1fc31e 100644 --- a/cache.h +++ b/cache.h @@ -709,7 +709,6 @@ int git_mkstemp(char *path, size_t n, const char *template); int git_mkstemps(char *path, size_t n, const char *template, int suffix_len); /* set default permissions by passing mode arguments to open(2) */ -int git_mkstemps_mode(char *pattern, int suffix_len, int mode); int git_mkstemp_mode(char *pattern, int mode); /* @@ -1147,7 +1146,6 @@ extern void alloc_report(void); /* trace.c */ __attribute__((format (printf, 1, 2))) extern void trace_printf(const char *format, ...); -extern void trace_vprintf(const char *key, const char *format, va_list ap); __attribute__((format (printf, 2, 3))) extern void trace_argv_printf(const char **argv, const char *format, ...); extern void trace_repo_setup(const char *prefix); diff --git a/convert.c b/convert.c index 3bb5a4d..fc74a6e 100644 --- a/convert.c +++ b/convert.c @@ -867,7 +867,7 @@ static struct stream_filter null_filter_singleton = { &null_vtbl, }; -int is_null_stream_filter(struct stream_filter *filter) +static int is_null_stream_filter(struct stream_filter *filter) { return filter == &null_filter_singleton; } diff --git a/convert.h b/convert.h index d799a165..3d464eb 100644 --- a/convert.h +++ b/convert.h @@ -51,7 +51,6 @@ struct stream_filter; /* opaque */ extern struct stream_filter *get_stream_filter(const char *path, const unsigned char *); extern void free_stream_filter(struct stream_filter *); -extern int is_null_stream_filter(struct stream_filter *); /* * Use as much input up to *isize_p and fill output up to *osize_p; diff --git a/diff.c b/diff.c index fcc0078..bd4be32 100644 --- a/diff.c +++ b/diff.c @@ -25,7 +25,7 @@ static int diff_detect_rename_default; static int diff_rename_limit_default = 400; static int diff_suppress_blank_empty; -int diff_use_color_default = -1; +static int diff_use_color_default = -1; static const char *diff_word_regex_cfg; static const char *external_diff_cmd_cfg; int diff_auto_refresh_index = 1; diff --git a/diff.h b/diff.h index 8c66b59..985bed4 100644 --- a/diff.h +++ b/diff.h @@ -230,7 +230,6 @@ extern int parse_long_opt(const char *opt, const char **argv, extern int git_diff_basic_config(const char *var, const char *value, void *cb); extern int git_diff_ui_config(const char *var, const char *value, void *cb); -extern int diff_use_color_default; extern void diff_setup(struct diff_options *); extern int diff_opt_parse(struct diff_options *, const char **, int); extern int diff_setup_done(struct diff_options *); diff --git a/dir.c b/dir.c index 08281d2..ebf1de6 100644 --- a/dir.c +++ b/dir.c @@ -19,7 +19,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, in static int get_dtype(struct dirent *de, const char *path, int len); /* helper string functions with support for the ignore_case flag */ -int strcmp_icase(const char *a, const char *b) +static int strcmp_icase(const char *a, const char *b) { return ignore_case ? strcasecmp(a, b) : strcmp(a, b); } @@ -29,7 +29,7 @@ int strncmp_icase(const char *a, const char *b, size_t count) return ignore_case ? strncasecmp(a, b, count) : strncmp(a, b, count); } -int fnmatch_icase(const char *pattern, const char *string, int flags) +static int fnmatch_icase(const char *pattern, const char *string, int flags) { return fnmatch(pattern, string, flags | (ignore_case ? FNM_CASEFOLD : 0)); } diff --git a/dir.h b/dir.h index 433b5b4..640b18a 100644 --- a/dir.h +++ b/dir.h @@ -106,8 +106,6 @@ extern int remove_dir_recursively(struct strbuf *path, int flag); /* tries to remove the path with empty directories along it, ignores ENOENT */ extern int remove_path(const char *path); -extern int strcmp_icase(const char *a, const char *b); extern int strncmp_icase(const char *a, const char *b, size_t count); -extern int fnmatch_icase(const char *pattern, const char *string, int flags); #endif diff --git a/git-compat-util.h b/git-compat-util.h index 5ef8ff7..3b5648d 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -330,7 +330,6 @@ extern char *gitmkdtemp(char *); #ifdef NO_MKSTEMPS #define mkstemps gitmkstemps -extern int gitmkstemps(char *, int); #endif #ifdef NO_UNSETENV @@ -566,10 +565,6 @@ void git_qsort(void *base, size_t nmemb, size_t size, */ int unlink_or_warn(const char *path); /* - * Likewise for rmdir(2). - */ -int rmdir_or_warn(const char *path); -/* * Calls the correct function out of {unlink,rmdir}_or_warn based on * the supplied file mode. */ diff --git a/graph.c b/graph.c index 7358416..e013289 100644 --- a/graph.c +++ b/graph.c @@ -62,7 +62,22 @@ enum graph_state { static const char **column_colors; static unsigned short column_colors_max; -void graph_set_column_colors(const char **colors, unsigned short colors_max) +/* + * Set up a custom scheme for column colors. + * + * The default column color scheme inserts ANSI color escapes to colorize + * the graph. The various color escapes are stored in an array of strings + * where each entry corresponds to a color, except for the last entry, + * which denotes the escape for resetting the color back to the default. + * When generating the graph, strings from this array are inserted before + * and after the various column characters. + * + * This function allows you to enable a custom array of color escapes. + * The 'colors_max' argument is the index of the last "reset" entry. + * + * This functions must be called BEFORE graph_init() is called. + */ +static void graph_set_column_colors(const char **colors, unsigned short colors_max) { column_colors = colors; column_colors_max = colors_max; @@ -1113,7 +1128,16 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct strbuf graph_update_state(graph, GRAPH_PADDING); } -int graph_next_line(struct git_graph *graph, struct strbuf *sb) +/* + * Output the next line for a graph. + * This formats the next graph line into the specified strbuf. It is not + * terminated with a newline. + * + * Returns 1 if the line includes the current commit, and 0 otherwise. + * graph_next_line() will return 1 exactly once for each time + * graph_update() is called. + */ +static int graph_next_line(struct git_graph *graph, struct strbuf *sb) { switch (graph->state) { case GRAPH_PADDING: diff --git a/graph.h b/graph.h index aff960c..5ddbb1a 100644 --- a/graph.h +++ b/graph.h @@ -5,23 +5,6 @@ struct git_graph; /* - * Set up a custom scheme for column colors. - * - * The default column color scheme inserts ANSI color escapes to colorize - * the graph. The various color escapes are stored in an array of strings - * where each entry corresponds to a color, except for the last entry, - * which denotes the escape for resetting the color back to the default. - * When generating the graph, strings from this array are inserted before - * and after the various column characters. - * - * This function allows you to enable a custom array of color escapes. - * The 'colors_max' argument is the index of the last "reset" entry. - * - * This functions must be called BEFORE graph_init() is called. - */ -void graph_set_column_colors(const char **colors, unsigned short colors_max); - -/* * Create a new struct git_graph. */ struct git_graph *graph_init(struct rev_info *opt); @@ -50,18 +33,6 @@ void graph_update(struct git_graph *graph, struct commit *commit); int graph_is_commit_finished(struct git_graph const *graph); /* - * Output the next line for a graph. - * This formats the next graph line into the specified strbuf. It is not - * terminated with a newline. - * - * Returns 1 if the line includes the current commit, and 0 otherwise. - * graph_next_line() will return 1 exactly once for each time - * graph_update() is called. - */ -int graph_next_line(struct git_graph *graph, struct strbuf *sb); - - -/* * graph_show_*: helper functions for printing to stdout */ diff --git a/notes.c b/notes.c index 93e9868..d98243f 100644 --- a/notes.c +++ b/notes.c @@ -1196,8 +1196,19 @@ void free_notes(struct notes_tree *t) memset(t, 0, sizeof(struct notes_tree)); } -void format_note(struct notes_tree *t, const unsigned char *object_sha1, - struct strbuf *sb, const char *output_encoding, int flags) + +/* + * Fill the given strbuf with the notes associated with the given object. + * + * If the given notes_tree structure is not initialized, it will be auto- + * initialized to the default value (see documentation for init_notes() above). + * If the given notes_tree is NULL, the internal/default notes_tree will be + * used instead. + * + * 'flags' is a bitwise combination of the above formatting flags. + */ +static void format_note(struct notes_tree *t, const unsigned char *object_sha1, + struct strbuf *sb, const char *output_encoding, int flags) { static const char utf8[] = "utf-8"; const unsigned char *sha1; diff --git a/notes.h b/notes.h index c716694..3592b19 100644 --- a/notes.h +++ b/notes.h @@ -241,20 +241,6 @@ void free_notes(struct notes_tree *t); #define NOTES_SHOW_HEADER 1 #define NOTES_INDENT 2 -/* - * Fill the given strbuf with the notes associated with the given object. - * - * If the given notes_tree structure is not initialized, it will be auto- - * initialized to the default value (see documentation for init_notes() above). - * If the given notes_tree is NULL, the internal/default notes_tree will be - * used instead. - * - * 'flags' is a bitwise combination of the above formatting flags. - */ -void format_note(struct notes_tree *t, const unsigned char *object_sha1, - struct strbuf *sb, const char *output_encoding, int flags); - - struct string_list; struct display_notes_opt { diff --git a/parse-options.c b/parse-options.c index 503ab5d..db80155 100644 --- a/parse-options.c +++ b/parse-options.c @@ -11,7 +11,7 @@ static int parse_options_usage(struct parse_opt_ctx_t *ctx, #define OPT_SHORT 1 #define OPT_UNSET 2 -int optbug(const struct option *opt, const char *reason) +static int optbug(const struct option *opt, const char *reason) { if (opt->long_name) return error("BUG: option '%s' %s", opt->long_name, reason); diff --git a/parse-options.h b/parse-options.h index 59e0b52..eed93cb 100644 --- a/parse-options.h +++ b/parse-options.h @@ -165,7 +165,6 @@ extern NORETURN void usage_msg_opt(const char *msg, const char * const *usagestr, const struct option *options); -extern int optbug(const struct option *opt, const char *reason); extern int opterror(const struct option *opt, const char *reason, int flags); /*----- incremental advanced APIs -----*/ diff --git a/rerere.c b/rerere.c index dcb525a..a2e36de 100644 --- a/rerere.c +++ b/rerere.c @@ -25,7 +25,7 @@ const char *rerere_path(const char *hex, const char *file) return git_path("rr-cache/%s/%s", hex, file); } -int has_rerere_resolution(const char *hex) +static int has_rerere_resolution(const char *hex) { struct stat st; return !stat(rerere_path(hex, "postimage"), &st); diff --git a/rerere.h b/rerere.h index fcd8bc1..156d2aa 100644 --- a/rerere.h +++ b/rerere.h @@ -16,7 +16,6 @@ extern void *RERERE_RESOLVED; extern int setup_rerere(struct string_list *, int); extern int rerere(int); extern const char *rerere_path(const char *hex, const char *file); -extern int has_rerere_resolution(const char *hex); extern int rerere_forget(const char **); extern int rerere_remaining(struct string_list *); extern void rerere_clear(struct string_list *); diff --git a/sha1-array.c b/sha1-array.c index b2f47f9..6f4a224 100644 --- a/sha1-array.c +++ b/sha1-array.c @@ -14,7 +14,7 @@ static int void_hashcmp(const void *a, const void *b) return hashcmp(a, b); } -void sha1_array_sort(struct sha1_array *array) +static void sha1_array_sort(struct sha1_array *array) { qsort(array->sha1, array->nr, sizeof(*array->sha1), void_hashcmp); array->sorted = 1; diff --git a/sha1-array.h b/sha1-array.h index 4499b5d..72bb33b 100644 --- a/sha1-array.h +++ b/sha1-array.h @@ -11,7 +11,6 @@ struct sha1_array { #define SHA1_ARRAY_INIT { NULL, 0, 0, 0 } void sha1_array_append(struct sha1_array *array, const unsigned char *sha1); -void sha1_array_sort(struct sha1_array *array); int sha1_array_lookup(struct sha1_array *array, const unsigned char *sha1); void sha1_array_clear(struct sha1_array *array); diff --git a/trace.c b/trace.c index d953416..5ec0e3b 100644 --- a/trace.c +++ b/trace.c @@ -62,7 +62,7 @@ static int get_trace_fd(const char *key, int *need_close) static const char err_msg[] = "Could not trace into fd given by " "GIT_TRACE environment variable"; -void trace_vprintf(const char *key, const char *fmt, va_list ap) +static void trace_vprintf(const char *key, const char *fmt, va_list ap) { struct strbuf buf = STRBUF_INIT; diff --git a/wrapper.c b/wrapper.c index 85f09df..390a7ae 100644 --- a/wrapper.c +++ b/wrapper.c @@ -236,29 +236,12 @@ int git_mkstemp(char *path, size_t len, const char *template) return mkstemp(path); } -/* git_mkstemps() - create tmp file with suffix honoring TMPDIR variable. */ -int git_mkstemps(char *path, size_t len, const char *template, int suffix_len) -{ - const char *tmp; - size_t n; - - tmp = getenv("TMPDIR"); - if (!tmp) - tmp = "/tmp"; - n = snprintf(path, len, "%s/%s", tmp, template); - if (len <= n) { - errno = ENAMETOOLONG; - return -1; - } - return mkstemps(path, suffix_len); -} - /* Adapted from libiberty's mkstemp.c. */ #undef TMP_MAX #define TMP_MAX 16384 -int git_mkstemps_mode(char *pattern, int suffix_len, int mode) +static int git_mkstemps_mode(char *pattern, int suffix_len, int mode) { static const char letters[] = "abcdefghijklmnopqrstuvwxyz" @@ -327,11 +310,28 @@ int git_mkstemp_mode(char *pattern, int mode) return git_mkstemps_mode(pattern, 0, mode); } -int gitmkstemps(char *pattern, int suffix_len) +static int gitmkstemps(char *pattern, int suffix_len) { return git_mkstemps_mode(pattern, suffix_len, 0600); } +/* git_mkstemps() - create tmp file with suffix honoring TMPDIR variable. */ +int git_mkstemps(char *path, size_t len, const char *template, int suffix_len) +{ + const char *tmp; + size_t n; + + tmp = getenv("TMPDIR"); + if (!tmp) + tmp = "/tmp"; + n = snprintf(path, len, "%s/%s", tmp, template); + if (len <= n) { + errno = ENAMETOOLONG; + return -1; + } + return mkstemps(path, suffix_len); +} + int xmkstemp_mode(char *template, int mode) { int fd; @@ -372,7 +372,7 @@ int unlink_or_warn(const char *file) return warn_if_unremovable("unlink", file, unlink(file)); } -int rmdir_or_warn(const char *file) +static int rmdir_or_warn(const char *file) { return warn_if_unremovable("rmdir", file, rmdir(file)); } -- 1.7.7.rc0.188.g3793ac ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] make-static: master 2011-09-11 21:38 ` [PATCH 1/3] make-static: master Junio C Hamano @ 2011-09-12 6:14 ` Ramkumar Ramachandra 2011-09-15 4:29 ` Junio C Hamano 2011-09-13 22:51 ` Ramsay Jones 2011-09-14 8:52 ` Thomas Rast 2 siblings, 1 reply; 14+ messages in thread From: Ramkumar Ramachandra @ 2011-09-12 6:14 UTC (permalink / raw) To: Junio C Hamano; +Cc: Ramsay Jones, GIT Mailing-list Hi Junio, Junio C Hamano writes: > Many symbols that are exported to the global scope do not have to be. > > Signed-off-by: Junio C Hamano <junio@pobox.com> > --- > * To be applied on top of 3793ac5 (RelNotes/1.7.7: minor fixes, 2011-09-07) > [...] Awesome! I've seen many similar "make-static" patches come up on the list, but turned down due to code churn issues. I'm happy to finally see it being merged. What does this mean: should we try to get such things merged immediately after a release? On a related note: I'd also like to know if we can do anything about restructuring the source tree (moving files around, renaming etc). Minor nit: I can see commits with "Author: Junio C Hamano <gitster@pobox.com>", and "Signed-off-by: Junio C Hamano <junio@pobox.com>" on both 'pu' and 'next'. They seem to be different people at first glance :) Thanks. -- Ram ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] make-static: master 2011-09-12 6:14 ` Ramkumar Ramachandra @ 2011-09-15 4:29 ` Junio C Hamano 0 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2011-09-15 4:29 UTC (permalink / raw) To: Ramkumar Ramachandra; +Cc: Ramsay Jones, GIT Mailing-list Ramkumar Ramachandra <artagnon@gmail.com> writes: > Junio C Hamano writes: >> Many symbols that are exported to the global scope do not have to be. >> >> Signed-off-by: Junio C Hamano <junio@pobox.com> >> --- >> To be applied on top of 3793ac5 (RelNotes/1.7.7: minor fixes, 2011-09-07) >> [...] > > Awesome! I've seen many similar "make-static" patches come up on the > list, but turned down due to code churn issues. I'm happy to finally > see it being merged. Not so fast. We just got a new series from Brandon Casey that adds a new caller to one of the functions that were unused outside a file. Seeing that this happens even inside a relative calm of pre-release feature and code freeze, realize why we call such a change a "churn" and try to avoid it, and imagine how much damage it would have caused if more topics were actively being updated. You should learn to tame your enthusiasm ;-) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] make-static: master 2011-09-11 21:38 ` [PATCH 1/3] make-static: master Junio C Hamano 2011-09-12 6:14 ` Ramkumar Ramachandra @ 2011-09-13 22:51 ` Ramsay Jones 2011-09-13 23:46 ` Junio C Hamano 2011-09-14 8:52 ` Thomas Rast 2 siblings, 1 reply; 14+ messages in thread From: Ramsay Jones @ 2011-09-13 22:51 UTC (permalink / raw) To: Junio C Hamano; +Cc: GIT Mailing-list Junio C Hamano wrote: > Many symbols that are exported to the global scope do not have to be. > > Signed-off-by: Junio C Hamano <junio@pobox.com> > --- > * To be applied on top of 3793ac5 (RelNotes/1.7.7: minor fixes, 2011-09-07) > [snipped patch] commit f34196da7b55cbf9f2651e095b6559430aff0baf (make-static: master, 11-09-2011) in the next branch (at repo.or.cz), but *not* this patch, breaks the build on cygwin. The failure is caused by this part of the commit: diff --git a/environment.c b/environment.c index e96edcf..478f2afa 100644 --- a/environment.c +++ b/environment.c @@ -147,11 +147,6 @@ int is_bare_repository(void) return is_bare_repository_cfg && !get_git_work_tree(); } -int have_git_dir(void) -{ - return !!git_dir; -} - const char *get_git_dir(void) { if (!git_dir) since have_git_dir() is used in compat/cygwin.c (line 117). ATB, Ramsay Jones ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] make-static: master 2011-09-13 22:51 ` Ramsay Jones @ 2011-09-13 23:46 ` Junio C Hamano 2011-09-14 6:50 ` Johannes Sixt 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2011-09-13 23:46 UTC (permalink / raw) To: Ramsay Jones; +Cc: GIT Mailing-list Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes: > Junio C Hamano wrote: >> Many symbols that are exported to the global scope do not have to be. >> >> Signed-off-by: Junio C Hamano <junio@pobox.com> >> --- >> * To be applied on top of 3793ac5 (RelNotes/1.7.7: minor fixes, 2011-09-07) >> > [snipped patch] > > commit f34196da7b55cbf9f2651e095b6559430aff0baf (make-static: master, 11-09-2011) > in the next branch (at repo.or.cz), but *not* this patch, breaks the build on > cygwin. Thanks. This kind of breakage report was exactly what I was looking for by merging it early to 'next'. Hopefully no other (function / platform) combo has such dependencies... environment.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/environment.c b/environment.c index 478f2afa..e810f6b 100644 --- a/environment.c +++ b/environment.c @@ -147,6 +147,15 @@ int is_bare_repository(void) return is_bare_repository_cfg && !get_git_work_tree(); } +/* + * This symbol might be unreferenced in normal builds but + * compat/cygwin.c refers to it. Do not remove without checking! + */ +int have_git_dir(void) +{ + return !!git_dir; +} + const char *get_git_dir(void) { if (!git_dir) ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] make-static: master 2011-09-13 23:46 ` Junio C Hamano @ 2011-09-14 6:50 ` Johannes Sixt 2011-09-14 19:03 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Johannes Sixt @ 2011-09-14 6:50 UTC (permalink / raw) To: Junio C Hamano; +Cc: Ramsay Jones, GIT Mailing-list Am 9/14/2011 1:46, schrieb Junio C Hamano: > This kind of breakage report was exactly what I was looking for by > merging it early to 'next'. Hopefully no other (function / platform) combo > has such dependencies... There is! prepare_git_cmd is used from the Windows section in run-command.c. Therefore, the following hunks of the patch should be reverted. diff --git a/exec_cmd.c b/exec_cmd.c index 171e841..1dddbf4 100644 --- a/exec_cmd.c +++ b/exec_cmd.c @@ -113,7 +113,7 @@ void setup_path(void) strbuf_release(&new_path); } -const char **prepare_git_cmd(const char **argv) +static const char **prepare_git_cmd(const char **argv) { int argc; const char **nargv; diff --git a/exec_cmd.h b/exec_cmd.h index e2b546b..f5d2cdd 100644 --- a/exec_cmd.h +++ b/exec_cmd.h @@ -5,7 +5,6 @@ extern void git_set_argv_exec_path(const char *exec_path); extern const char *git_extract_argv0_path(const char *path); extern const char *git_exec_path(void); extern void setup_path(void); -extern const char **prepare_git_cmd(const char **argv); extern int execv_git_cmd(const char **argv); /* NULL terminated */ extern int execl_git_cmd(const char *cmd, ...); extern const char *system_path(const char *path); ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] make-static: master 2011-09-14 6:50 ` Johannes Sixt @ 2011-09-14 19:03 ` Junio C Hamano 2011-09-14 20:33 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2011-09-14 19:03 UTC (permalink / raw) To: Johannes Sixt; +Cc: Ramsay Jones, GIT Mailing-list Johannes Sixt <j.sixt@viscovery.net> writes: > Am 9/14/2011 1:46, schrieb Junio C Hamano: >> This kind of breakage report was exactly what I was looking for by >> merging it early to 'next'. Hopefully no other (function / platform) combo >> has such dependencies... > > There is! prepare_git_cmd is used from the Windows section in run-command.c. > Therefore, the following hunks of the patch should be reverted. Sorry, I do not think it should just be "reverted". Instead, a new comment should be added to explain why it needs to be exposed, which was the _whole_ point of the message you are replying to, in which I showed how it should be done by an example. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] make-static: master 2011-09-14 19:03 ` Junio C Hamano @ 2011-09-14 20:33 ` Junio C Hamano 0 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2011-09-14 20:33 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Sixt, Ramsay Jones, GIT Mailing-list Junio C Hamano <gitster@pobox.com> writes: > Johannes Sixt <j.sixt@viscovery.net> writes: > >> Am 9/14/2011 1:46, schrieb Junio C Hamano: >>> This kind of breakage report was exactly what I was looking for by >>> merging it early to 'next'. Hopefully no other (function / platform) combo >>> has such dependencies... >> >> There is! prepare_git_cmd is used from the Windows section in run-command.c. >> Therefore, the following hunks of the patch should be reverted. Thanks; this fix-up will be queued on top. -- >8 -- Subject: [PATCH] exec_cmd.c: prepare_git_cmd() is sometimes used Add warning to prevent people from making the same mistake. Noticed by Johannes Sixt. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- exec_cmd.c | 7 ++++++- exec_cmd.h | 1 + 2 files changed, 7 insertions(+), 1 deletions(-) diff --git a/exec_cmd.c b/exec_cmd.c index 1dddbf4..9c784db 100644 --- a/exec_cmd.c +++ b/exec_cmd.c @@ -113,7 +113,12 @@ void setup_path(void) strbuf_release(&new_path); } -static const char **prepare_git_cmd(const char **argv) +/* + * This symbol may be unreferenced from outside this file in some + * builds, but run-command.c on Windows does use it. Do not make it + * static without checking! + */ +const char **prepare_git_cmd(const char **argv) { int argc; const char **nargv; diff --git a/exec_cmd.h b/exec_cmd.h index f5d2cdd..e2b546b 100644 --- a/exec_cmd.h +++ b/exec_cmd.h @@ -5,6 +5,7 @@ extern void git_set_argv_exec_path(const char *exec_path); extern const char *git_extract_argv0_path(const char *path); extern const char *git_exec_path(void); extern void setup_path(void); +extern const char **prepare_git_cmd(const char **argv); extern int execv_git_cmd(const char **argv); /* NULL terminated */ extern int execl_git_cmd(const char *cmd, ...); extern const char *system_path(const char *path); -- 1.7.7.rc1.1.g1e5814 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] make-static: master 2011-09-11 21:38 ` [PATCH 1/3] make-static: master Junio C Hamano 2011-09-12 6:14 ` Ramkumar Ramachandra 2011-09-13 22:51 ` Ramsay Jones @ 2011-09-14 8:52 ` Thomas Rast 2 siblings, 0 replies; 14+ messages in thread From: Thomas Rast @ 2011-09-14 8:52 UTC (permalink / raw) To: Junio C Hamano; +Cc: Ramsay Jones, GIT Mailing-list Junio C Hamano wrote: > diff --git a/graph.h b/graph.h [...] > @@ -50,18 +33,6 @@ void graph_update(struct git_graph *graph, struct commit *commit); > int graph_is_commit_finished(struct git_graph const *graph); > > /* > - * Output the next line for a graph. > - * This formats the next graph line into the specified strbuf. It is not > - * terminated with a newline. > - * > - * Returns 1 if the line includes the current commit, and 0 otherwise. > - * graph_next_line() will return 1 exactly once for each time > - * graph_update() is called. > - */ > -int graph_next_line(struct git_graph *graph, struct strbuf *sb); Well, you removed that, but you still have /* * Determine if a graph has finished outputting lines for the current * commit. * * Returns 1 if graph_next_line() needs to be called again before * graph_update() should be called. Returns 0 if no more lines are needed * for this commit. If 0 is returned, graph_next_line() may still be * called without calling graph_update(), and it will merely output * appropriate "vertical padding" in the graph. */ int graph_is_commit_finished(struct git_graph const *graph); which pretty explicitly tells the user to call graph_next_line in conjunction with graph_update, even though we currently don't have any callers outside graph.c doing that. (I can't really say whether the API would be better off without graph_next_line exposed; I just noticed because I drag along the line-log stuff and it uses graph_next_line.) -- Thomas Rast trast@{inf,student}.ethz.ch ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] make-static: next 2011-09-11 19:38 [PATCH 1/3] sparse: Fix some "symbol not declared" warnings Ramsay Jones 2011-09-11 21:38 ` Junio C Hamano 2011-09-11 21:38 ` [PATCH 1/3] make-static: master Junio C Hamano @ 2011-09-11 21:38 ` Junio C Hamano 2011-09-11 21:38 ` [PATCH 3/3] make-static: pu Junio C Hamano 3 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2011-09-11 21:38 UTC (permalink / raw) To: Ramsay Jones; +Cc: GIT Mailing-list Many symbols that are exported to the global scope do not have to be. This needs to be split into separate patches and queued on top of each individual topic. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- builtin/revert.c | 2 +- credential.c | 46 +++++++++++++++++++++++----------------------- credential.h | 1 - submodule.c | 4 ++-- 4 files changed, 26 insertions(+), 27 deletions(-) diff --git a/builtin/revert.c b/builtin/revert.c index 5e97622..515f3cc 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -659,7 +659,7 @@ static void read_and_refresh_cache(struct replay_opts *opts) * assert(commit_list_count(list) == 2); * return list; */ -struct commit_list **commit_list_append(struct commit *commit, +static struct commit_list **commit_list_append(struct commit *commit, struct commit_list **next) { struct commit_list *new = xmalloc(sizeof(struct commit_list)); diff --git a/credential.c b/credential.c index 7a0c751..6243b03 100644 --- a/credential.c +++ b/credential.c @@ -171,6 +171,29 @@ static int credential_do(struct credential *c, const char *method, return r; } +static int credential_fill_gently(struct credential *c, + const struct string_list *methods) +{ + int i; + + if (c->username && c->password) + return 0; + + if (!methods) + methods = &default_methods; + + if (!methods->nr) + return credential_getpass(c); + + for (i = 0; i < methods->nr; i++) { + if (!credential_do(c, methods->items[i].string, NULL) && + c->username && c->password) + return 0; + } + + return -1; +} + void credential_fill(struct credential *c, const struct string_list *methods) { struct strbuf err = STRBUF_INIT; @@ -195,29 +218,6 @@ void credential_fill(struct credential *c, const struct string_list *methods) die("%s", err.buf); } -int credential_fill_gently(struct credential *c, - const struct string_list *methods) -{ - int i; - - if (c->username && c->password) - return 0; - - if (!methods) - methods = &default_methods; - - if (!methods->nr) - return credential_getpass(c); - - for (i = 0; i < methods->nr; i++) { - if (!credential_do(c, methods->items[i].string, NULL) && - c->username && c->password) - return 0; - } - - return -1; -} - void credential_reject(struct credential *c, const struct string_list *methods) { int i; diff --git a/credential.h b/credential.h index 788ed8e..07541ad 100644 --- a/credential.h +++ b/credential.h @@ -13,7 +13,6 @@ struct string_list; int credential_getpass(struct credential *); void credential_from_config(struct credential *); -int credential_fill_gently(struct credential *, const struct string_list *methods); void credential_fill(struct credential *, const struct string_list *methods); void credential_reject(struct credential *, const struct string_list *methods); diff --git a/submodule.c b/submodule.c index 38d9877..5a02890 100644 --- a/submodule.c +++ b/submodule.c @@ -318,7 +318,7 @@ static int has_remote(const char *refname, const unsigned char *sha1, int flags, return 1; } -int submodule_needs_pushing(const char *path, const unsigned char sha1[20], void *data) +static int submodule_needs_pushing(const char *path, const unsigned char sha1[20], void *data) { int *needs_pushing = data; @@ -352,7 +352,7 @@ int submodule_needs_pushing(const char *path, const unsigned char sha1[20], void return 1; } -int push_submodule(const char *path, const unsigned char sha1[20], void *data) +static int push_submodule(const char *path, const unsigned char sha1[20], void *data) { if (add_submodule_odb(path) || !lookup_commit_reference(sha1)) return 1; -- 1.7.7.rc0.188.g3793ac ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] make-static: pu 2011-09-11 19:38 [PATCH 1/3] sparse: Fix some "symbol not declared" warnings Ramsay Jones ` (2 preceding siblings ...) 2011-09-11 21:38 ` [PATCH 2/3] make-static: next Junio C Hamano @ 2011-09-11 21:38 ` Junio C Hamano 3 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2011-09-11 21:38 UTC (permalink / raw) To: Ramsay Jones; +Cc: GIT Mailing-list Many symbols that are exported to the global scope do not have to be. This needs to be split into separate patches and squashed into each individual commit that introduces these symbols. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- dir.c | 2 +- dir.h | 1 - metadata-cache.c | 8 ++++---- metadata-cache.h | 5 ----- refs.c | 2 +- 5 files changed, 6 insertions(+), 12 deletions(-) diff --git a/dir.c b/dir.c index fba6433..bee7ed1 100644 --- a/dir.c +++ b/dir.c @@ -34,7 +34,7 @@ static int fnmatch_icase(const char *pattern, const char *string, int flags) return fnmatch(pattern, string, flags | (ignore_case ? FNM_CASEFOLD : 0)); } -size_t common_prefix_len(const char **pathspec) +static size_t common_prefix_len(const char **pathspec) { const char *n, *first; size_t max = 0; diff --git a/dir.h b/dir.h index 592ceca..9b7bfd4 100644 --- a/dir.h +++ b/dir.h @@ -64,7 +64,6 @@ struct dir_struct { #define MATCHED_RECURSIVELY 1 #define MATCHED_FNMATCH 2 #define MATCHED_EXACTLY 3 -extern size_t common_prefix_len(const char **pathspec); extern char *common_prefix(const char **pathspec); extern int match_pathspec(const char **pathspec, const char *name, int namelen, int prefix, char *seen); extern int match_pathspec_depth(const struct pathspec *pathspec, diff --git a/metadata-cache.c b/metadata-cache.c index 32d3c21..5f5ca4e 100644 --- a/metadata-cache.c +++ b/metadata-cache.c @@ -287,8 +287,8 @@ static void *lookup_disk(struct metadata_cache *c, return c->disk_entries + (pos * record_size(c)) + 20; } -const void *metadata_cache_lookup(struct metadata_cache *c, - const struct object *obj) +static const void *metadata_cache_lookup(struct metadata_cache *c, + const struct object *obj) { void *r; @@ -300,8 +300,8 @@ const void *metadata_cache_lookup(struct metadata_cache *c, return r; } -void metadata_cache_add(struct metadata_cache *c, const struct object *obj, - const void *value) +static void metadata_cache_add(struct metadata_cache *c, const struct object *obj, + const void *value) { metadata_cache_init(c); add_decoration_value(&c->mem, obj, value, NULL); diff --git a/metadata-cache.h b/metadata-cache.h index 15484b5..4819563 100644 --- a/metadata-cache.h +++ b/metadata-cache.h @@ -24,11 +24,6 @@ struct metadata_cache { #define METADATA_CACHE_INIT(name, width, validity) \ { validity, { (name), (width) } } -const void *metadata_cache_lookup(struct metadata_cache *, - const struct object *); -void metadata_cache_add(struct metadata_cache *, const struct object *, - const void *value); - /* Convenience wrappers around metadata_cache_{lookup,add} */ int metadata_cache_lookup_uint32(struct metadata_cache *, const struct object *, diff --git a/refs.c b/refs.c index 1c4aa33..6e9588b 100644 --- a/refs.c +++ b/refs.c @@ -185,7 +185,7 @@ static void clear_cached_refs(struct cached_refs *ca) ca->did_loose = ca->did_packed = 0; } -struct cached_refs *create_cached_refs(const char *submodule) +static struct cached_refs *create_cached_refs(const char *submodule) { int len; struct cached_refs *refs; -- 1.7.7.rc0.188.g3793ac ^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2011-09-15 4:30 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-09-11 19:38 [PATCH 1/3] sparse: Fix some "symbol not declared" warnings Ramsay Jones 2011-09-11 21:38 ` Junio C Hamano 2011-09-12 0:06 ` Junio C Hamano 2011-09-11 21:38 ` [PATCH 1/3] make-static: master Junio C Hamano 2011-09-12 6:14 ` Ramkumar Ramachandra 2011-09-15 4:29 ` Junio C Hamano 2011-09-13 22:51 ` Ramsay Jones 2011-09-13 23:46 ` Junio C Hamano 2011-09-14 6:50 ` Johannes Sixt 2011-09-14 19:03 ` Junio C Hamano 2011-09-14 20:33 ` Junio C Hamano 2011-09-14 8:52 ` Thomas Rast 2011-09-11 21:38 ` [PATCH 2/3] make-static: next Junio C Hamano 2011-09-11 21:38 ` [PATCH 3/3] make-static: pu Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).