[PATCH v3 00/27] Compile with `-Wwrite-strings`
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Jun 3, 2024, 09:38 UTC
- Message-ID
- <cover.1717402403.git.ps@pks.im>
- In-Reply-To
- <cover.1716983704.git.ps@pks.im>
Hi,
this is the third version of my patch series that prepares the Git codebase to compile with `-Wwrite-strings`. This compiler warning changes the type of string constants from `char []` to `const char []`, which helps to identify cases where we may accidentally write to or free such constants.
Changes compared to v2:
- I have split up the second patch into multiple patches. This was
done so that we can have a deeper look at the respective sites that
need adjustment. - I dropped the local string arrays that I used in v2 and instead now
use string constants with casts in some places where we expect that
those should never be written to. This has the benefit that we would
segfault in case that expectation is violated, instead of silently
allowing a write to those local arrays. - I adapted multiple commit messages to not only mention freeing of
string constants, but also that writing to string constants is
illegal.Due to the split-up patch the range-diff got quite messy and was barely readable anymore. I thus included an interdiff instead, which should be way easier to read.
I realize that this patch series has grown quite large. Please let me know in case I shall split it up into multiple patch series.
Patrick
Patrick Steinhardt (27): global: improve const correctness when assigning string constants global: convert intentionally-leaking config strings to consts refs/reftable: stop micro-optimizing refname allocations on copy reftable: cast away constness when assigning constants to records refspec: remove global tag refspec structure builtin/remote: cast away constness in `get_head_names()` diff: cast string constant in `fill_textconv()` line-log: stop assigning string constant to file parent buffer line-log: always allocate the output prefix entry: refactor how we remove items for delayed checkouts ident: add casts for fallback name and GECOS object-file: mark cached object buffers as const object-file: make `buf` parameter of `index_mem()` a constant pretty: add casts for decoration option pointers compat/win32: fix const-correctness with string constants http: do not assign string constant to non-const field parse-options: cast long name for OPTION_ALIAS send-pack: always allocate receive status remote-curl: avoid assigning string constant to non-const variable revision: always store allocated strings in output encoding mailmap: always store allocated strings in mailmap blob imap-send: drop global `imap_server_conf` variable imap-send: fix leaking memory in `imap_server_conf` builtin/rebase: do not assign default backend to non-constant field builtin/rebase: always store allocated string in `options.strategy` builtin/merge: always store allocated strings in `pull_twohead` config.mak.dev: enable `-Wwrite-strings` warning
builtin/bisect.c | 3 +- builtin/blame.c | 2 +- builtin/bugreport.c | 2 +- builtin/check-ignore.c | 4 +- builtin/clone.c | 14 ++-- builtin/commit.c | 6 +- builtin/diagnose.c | 2 +- builtin/fetch.c | 11 ++- builtin/log.c | 2 +- builtin/mailsplit.c | 4 +- builtin/merge.c | 18 +++-- builtin/pull.c | 52 +++++++------- builtin/rebase.c | 81 ++++++++++++---------- builtin/receive-pack.c | 4 +- builtin/remote.c | 12 ++-- builtin/revert.c | 2 +- builtin/send-pack.c | 2 + compat/basename.c | 18 ++++- compat/mingw.c | 28 ++++---- compat/regex/regcomp.c | 2 +- compat/winansi.c | 2 +- config.mak.dev | 1 + diff.c | 6 +- diffcore-rename.c | 6 +- entry.c | 14 ++-- fmt-merge-msg.c | 2 +- fsck.c | 2 +- fsck.h | 2 +- gpg-interface.c | 6 +- http-backend.c | 2 +- http.c | 5 +- ident.c | 4 +- imap-send.c | 130 ++++++++++++++++++++--------------- line-log.c | 22 +++--- mailmap.c | 2 +- merge-ll.c | 11 ++- object-file.c | 23 ++++--- parse-options.h | 2 +- pretty.c | 6 +- refs.c | 2 +- refs.h | 2 +- refs/reftable-backend.c | 28 ++++---- refspec.c | 13 ---- refspec.h | 1 - reftable/basics.c | 15 ++-- reftable/basics.h | 4 +- reftable/basics_test.c | 4 +- reftable/block_test.c | 2 +- reftable/merged_test.c | 44 ++++++------ reftable/readwrite_test.c | 32 ++++----- reftable/record.c | 6 +- reftable/stack.c | 10 +-- reftable/stack_test.c | 56 +++++++-------- remote-curl.c | 53 +++++++------- revision.c | 3 +- run-command.c | 2 +- send-pack.c | 2 +- t/helper/test-hashmap.c | 3 +- t/helper/test-json-writer.c | 10 +-- t/helper/test-regex.c | 4 +- t/helper/test-rot13-filter.c | 5 +- t/t3900-i18n-commit.sh | 1 + t/t3901-i18n-patch.sh | 1 + t/unit-tests/t-strbuf.c | 10 +-- trailer.c | 2 +- userdiff.c | 10 +-- userdiff.h | 12 ++-- wt-status.c | 2 +- 68 files changed, 470 insertions(+), 386 deletions(-)
Interdiff against v2:
diff --git a/builtin/remote.c b/builtin/remote.c index 0324a5d48d..b44f580b8c 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -493,13 +493,13 @@ static int get_head_names(const struct ref *remote_refs, struct ref_states *stat { struct ref *ref, *matches; struct ref *fetch_map = NULL, **fetch_map_tail = &fetch_map; - struct refspec_item refspec; - char refspec_str[] = "refs/heads/*"; + struct refspec_item refspec = { + .force = 0, + .pattern = 1, + .src = (char *) "refs/heads/*", + .dst = (char *) "refs/heads/*", + }; - memset(&refspec, 0, sizeof(refspec)); - refspec.force = 0; - refspec.pattern = 1; - refspec.src = refspec.dst = refspec_str; get_fetch_map(remote_refs, &refspec, &fetch_map_tail, 0); matches = guess_remote_head(find_ref_by_name(remote_refs, "HEAD"), fetch_map, 1); @@ -508,7 +508,6 @@ static int get_head_names(const struct ref *remote_refs, struct ref_states *stat free_refs(fetch_map); free_refs(matches); - return 0; } diff --git a/compat/basename.c b/compat/basename.c index c3c9d65fac..5aa320306b 100644 --- a/compat/basename.c +++ b/compat/basename.c @@ -1,11 +1,6 @@ #include "../git-compat-util.h" #include "../strbuf.h" -/* - * Both basename(3P) and dirname(3P) are mis-specified because they return a - * non-constant pointer even though it is specified that they may return a - * pointer to internal memory. This variable here is a result of that. - */ static char current_directory[] = "."; /* Adapted from libiberty's basename.c. */ @@ -17,7 +12,13 @@ char *gitbasename (char *path) skip_dos_drive_prefix(&path); if (!path || !*path) - return current_directory; + /* + * basename(3P) is mis-specified because it returns a + * non-constant pointer even though it is specified to return a + * pointer to internal memory at times. The cast is a result of + * that. + */ + return (char *) ""; for (base = path; *path; path++) { if (!is_dir_sep(*path)) @@ -40,12 +41,14 @@ char *gitdirname(char *path) char *p = path, *slash = NULL, c; int dos_drive_prefix; - /* - * Same here, dirname(3P) is broken because it returns a non-constant - * pointer that may point to internal memory. - */ if (!p) - return current_directory; + /* + * dirname(3P) is mis-specified because it returns a + * non-constant pointer even though it is specified to return a + * pointer to internal memory at times. The cast is a result of + * that. + */ + return (char *) ""; if ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p) goto dot; diff --git a/compat/mingw.c b/compat/mingw.c index 60f0986f76..d378cd04cb 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -2257,7 +2257,6 @@ struct passwd *getpwuid(int uid) { static unsigned initialized; static char user_name[100]; - static char unknown[] = "unknown"; static struct passwd *p; wchar_t buf[100]; DWORD len; @@ -2280,7 +2279,11 @@ struct passwd *getpwuid(int uid) p->pw_name = user_name; p->pw_gecos = get_extended_user_info(NameDisplay); if (!p->pw_gecos) - p->pw_gecos = unknown; + /* + * Data returned by getpwuid(3P) is treated as internal and + * must never be written to or freed. + */ + p->pw_gecos = (char *) "unknown"; p->pw_dir = NULL; initialized = 1; diff --git a/diff.c b/diff.c index 1439a5a01d..cecda216cf 100644 --- a/diff.c +++ b/diff.c @@ -7231,12 +7231,11 @@ size_t fill_textconv(struct repository *r, struct diff_filespec *df, char **outbuf) { - static char empty_str[] = ""; size_t size; if (!driver) { if (!DIFF_FILE_VALID(df)) { - *outbuf = empty_str; + *outbuf = (char *) ""; return 0; } if (diff_populate_filespec(r, df, NULL)) diff --git a/entry.c b/entry.c index 2fc06ac90c..f291d8eee6 100644 --- a/entry.c +++ b/entry.c @@ -167,6 +167,11 @@ static int remove_available_paths(struct string_list_item *item, void *cb_data) return !available; } +static int string_is_not_null(struct string_list_item *item, void *data UNUSED) +{ + return !!item->string; +} + int finish_delayed_checkout(struct checkout *state, int show_progress) { int errs = 0; @@ -175,7 +180,6 @@ int finish_delayed_checkout(struct checkout *state, int show_progress) struct string_list_item *filter, *path; struct progress *progress = NULL; struct delayed_checkout *dco = state->delayed_checkout; - char empty_str[] = ""; if (!state->delayed_checkout) return errs; @@ -190,7 +194,7 @@ int finish_delayed_checkout(struct checkout *state, int show_progress) if (!async_query_available_blobs(filter->string, &available_paths)) { /* Filter reported an error */ errs = 1; - filter->string = empty_str; + filter->string = NULL; continue; } if (available_paths.nr <= 0) { @@ -200,7 +204,7 @@ int finish_delayed_checkout(struct checkout *state, int show_progress) * filter from the list (see * "string_list_remove_empty_items" call below). */ - filter->string = empty_str; + filter->string = NULL; continue; } @@ -226,7 +230,7 @@ int finish_delayed_checkout(struct checkout *state, int show_progress) * Do not ask the filter for available blobs, * again, as the filter is likely buggy. */ - filter->string = empty_str; + filter->string = NULL; continue; } ce = index_file_exists(state->istate, path->string, @@ -240,7 +244,8 @@ int finish_delayed_checkout(struct checkout *state, int show_progress) errs = 1; } } - string_list_remove_empty_items(&dco->filters, 0); + + filter_string_list(&dco->filters, 0, string_is_not_null, NULL); } stop_progress(&progress); string_list_clear(&dco->filters, 0); diff --git a/ident.c b/ident.c index df7aa42802..caf41fb2a9 100644 --- a/ident.c +++ b/ident.c @@ -46,14 +46,9 @@ static struct passwd *xgetpwuid_self(int *is_bogus) pw = getpwuid(getuid()); if (!pw) { static struct passwd fallback; - static char fallback_name[] = "unknown"; + fallback.pw_name = (char *) "unknown"; #ifndef NO_GECOS_IN_PWENT - static char fallback_gcos[] = "Unknown"; -#endif - - fallback.pw_name = fallback_name; -#ifndef NO_GECOS_IN_PWENT - fallback.pw_gecos = fallback_gcos; + fallback.pw_gecos = (char *) "Unknown"; #endif pw = &fallback; if (is_bogus) diff --git a/line-log.c b/line-log.c index 9a298209d0..67c80b39a0 100644 --- a/line-log.c +++ b/line-log.c @@ -1036,7 +1036,7 @@ static int process_diff_filepair(struct rev_info *rev, struct range_set tmp; struct diff_ranges diff; mmfile_t file_parent, file_target; - char empty_str[] = ""; + char *parent_data_to_free = NULL; assert(pair->two->path); while (rg) { @@ -1061,7 +1061,7 @@ static int process_diff_filepair(struct rev_info *rev, file_parent.ptr = pair->one->data; file_parent.size = pair->one->size; } else { - file_parent.ptr = empty_str; + file_parent.ptr = parent_data_to_free = xstrdup(""); file_parent.size = 0; } @@ -1080,6 +1080,7 @@ static int process_diff_filepair(struct rev_info *rev, diff_ranges_release(&diff); + free(parent_data_to_free); return ((*diff_out)->parent.nr > 0); } diff --git a/object-file.c b/object-file.c index 46ea00ac46..b5b5a59dc6 100644 --- a/object-file.c +++ b/object-file.c @@ -277,18 +277,17 @@ int hash_algo_by_length(int len) static struct cached_object { struct object_id oid; enum object_type type; - void *buf; + const void *buf; unsigned long size; } *cached_objects; static int cached_object_nr, cached_object_alloc; -static char empty_tree_buf[] = ""; static struct cached_object empty_tree = { .oid = { .hash = EMPTY_TREE_SHA1_BIN_LITERAL, }, .type = OBJ_TREE, - .buf = empty_tree_buf, + .buf = "", }; static struct cached_object *find_cached_object(const struct object_id *oid) @@ -1779,6 +1778,10 @@ int pretend_object_file(void *buf, unsigned long len, enum object_type type, struct object_id *oid) { struct cached_object *co; + char *co_buf; + + co_buf = xmalloc(len); + memcpy(co_buf, buf, len); hash_object_file(the_hash_algo, buf, len, type, oid); if (repo_has_object_file_with_flags(the_repository, oid, OBJECT_INFO_QUICK | OBJECT_INFO_SKIP_FETCH_OBJECT) || @@ -1788,8 +1791,7 @@ int pretend_object_file(void *buf, unsigned long len, enum object_type type, co = &cached_objects[cached_object_nr++]; co->size = len; co->type = type; - co->buf = xmalloc(len); - memcpy(co->buf, buf, len); + co->buf = co_buf; oidcpy(&co->oid, oid); return 0; } diff --git a/pretty.c b/pretty.c index 1a0030b32a..1df9d635fb 100644 --- a/pretty.c +++ b/pretty.c @@ -1583,10 +1583,9 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ return 1; case 'D': { - char empty_str[] = ""; const struct decoration_options opts = { - .prefix = empty_str, - .suffix = empty_str, + .prefix = (char *) "", + .suffix = (char *) "", }; format_decorations(sb, commit, c->auto_color, &opts); diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 1908e74dea..e77faa2b9d 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1285,7 +1285,6 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data) struct strbuf errbuf = STRBUF_INIT; size_t logs_nr = 0, logs_alloc = 0, i; const char *committer_info; - char head[] = "HEAD"; int ret; committer_info = git_committer_info(0); @@ -1341,10 +1340,10 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data) * old reference. */ refs[0] = old_ref; - refs[0].refname = (char *)arg->newname; + refs[0].refname = xstrdup(arg->newname); refs[0].update_index = creation_ts; if (arg->delete_old) { - refs[1].refname = (char *)arg->oldname; + refs[1].refname = xstrdup(arg->oldname); refs[1].value_type = REFTABLE_REF_DELETION; refs[1].update_index = deletion_ts; } @@ -1367,7 +1366,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data) ALLOC_GROW(logs, logs_nr + 1, logs_alloc); memset(&logs[logs_nr], 0, sizeof(logs[logs_nr])); fill_reftable_log_record(&logs[logs_nr], &committer_ident); - logs[logs_nr].refname = (char *)arg->newname; + logs[logs_nr].refname = xstrdup(arg->newname); logs[logs_nr].update_index = deletion_ts; logs[logs_nr].value.update.message = xstrndup(arg->logmsg, arg->refs->write_options.block_size / 2); @@ -1388,7 +1387,13 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data) if (append_head_reflog) { ALLOC_GROW(logs, logs_nr + 1, logs_alloc); logs[logs_nr] = logs[logs_nr - 1]; - logs[logs_nr].refname = head; + logs[logs_nr].refname = xstrdup("HEAD"); + logs[logs_nr].value.update.name = + xstrdup(logs[logs_nr].value.update.name); + logs[logs_nr].value.update.email = + xstrdup(logs[logs_nr].value.update.email); + logs[logs_nr].value.update.message = + xstrdup(logs[logs_nr].value.update.message); logs_nr++; } } @@ -1399,7 +1404,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data) ALLOC_GROW(logs, logs_nr + 1, logs_alloc); memset(&logs[logs_nr], 0, sizeof(logs[logs_nr])); fill_reftable_log_record(&logs[logs_nr], &committer_ident); - logs[logs_nr].refname = (char *)arg->newname; + logs[logs_nr].refname = xstrdup(arg->newname); logs[logs_nr].update_index = creation_ts; logs[logs_nr].value.update.message = xstrndup(arg->logmsg, arg->refs->write_options.block_size / 2); @@ -1431,7 +1436,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data) */ ALLOC_GROW(logs, logs_nr + 1, logs_alloc); logs[logs_nr] = old_log; - logs[logs_nr].refname = (char *)arg->newname; + logs[logs_nr].refname = xstrdup(arg->newname); logs_nr++; /* @@ -1440,7 +1445,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data) if (arg->delete_old) { ALLOC_GROW(logs, logs_nr + 1, logs_alloc); memset(&logs[logs_nr], 0, sizeof(logs[logs_nr])); - logs[logs_nr].refname = (char *)arg->oldname; + logs[logs_nr].refname = xstrdup(arg->oldname); logs[logs_nr].value_type = REFTABLE_LOG_DELETION; logs[logs_nr].update_index = old_log.update_index; logs_nr++; @@ -1463,13 +1468,11 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data) reftable_iterator_destroy(&it); string_list_clear(&skip, 0); strbuf_release(&errbuf); - for (i = 0; i < logs_nr; i++) { - if (logs[i].refname == head) - continue; - logs[i].refname = NULL; + for (i = 0; i < logs_nr; i++) reftable_log_record_release(&logs[i]); - } free(logs); + for (i = 0; i < ARRAY_SIZE(refs); i++) + reftable_ref_record_release(&refs[i]); reftable_ref_record_release(&old_ref); reftable_log_record_release(&old_log); return ret; diff --git a/reftable/basics.c b/reftable/basics.c index fea711db7e..0058619ca6 100644 --- a/reftable/basics.c +++ b/reftable/basics.c @@ -67,9 +67,9 @@ void free_names(char **a) reftable_free(a); } -size_t names_length(char **names) +size_t names_length(const char **names) { - char **p = names; + const char **p = names; while (*p) p++; return p - names; @@ -102,15 +102,12 @@ void parse_names(char *buf, int size, char ***namesp) *namesp = names; } -int names_equal(char **a, char **b) +int names_equal(const char **a, const char **b) { - int i = 0; - for (; a[i] && b[i]; i++) { - if (strcmp(a[i], b[i])) { + size_t i = 0; + for (; a[i] && b[i]; i++) + if (strcmp(a[i], b[i])) return 0; - } - } - return a[i] == b[i]; } diff --git a/reftable/basics.h b/reftable/basics.h index 523ecd5307..c8fec68d4e 100644 --- a/reftable/basics.h +++ b/reftable/basics.h @@ -42,10 +42,10 @@ void free_names(char **a); void parse_names(char *buf, int size, char ***namesp); /* compares two NULL-terminated arrays of strings. */ -int names_equal(char **a, char **b); +int names_equal(const char **a, const char **b); /* returns the array size of a NULL-terminated array of strings. */ -size_t names_length(char **names); +size_t names_length(const char **names); /* Allocation routines; they invoke the functions set through * reftable_set_alloc() */ diff --git a/reftable/basics_test.c b/reftable/basics_test.c index 23fab22eb1..13bc761817 100644 --- a/reftable/basics_test.c +++ b/reftable/basics_test.c @@ -58,8 +58,7 @@ static void test_binsearch(void) static void test_names_length(void) { - char a[] = "a", b[] = "b"; - char *names[] = { a, b, NULL }; + const char *names[] = { "a", "b", NULL }; EXPECT(names_length(names) == 2); } diff --git a/reftable/block_test.c b/reftable/block_test.c index d5967b214d..90aecd5a7c 100644 --- a/reftable/block_test.c +++ b/reftable/block_test.c @@ -19,7 +19,7 @@ license that can be found in the LICENSE file or at static void test_block_read_write(void) { const int header_off = 21; /* random */ - char *names[30], empty_str[] = ""; + char *names[30]; const int N = ARRAY_SIZE(names); const int block_size = 1024; struct reftable_block block = { NULL }; @@ -42,7 +42,7 @@ static void test_block_read_write(void) block_writer_init(&bw, BLOCK_TYPE_REF, block.data, block_size, header_off, hash_size(GIT_SHA1_FORMAT_ID)); - rec.u.ref.refname = empty_str; + rec.u.ref.refname = (char *) ""; rec.u.ref.value_type = REFTABLE_REF_DELETION; n = block_writer_add(&bw, &rec); EXPECT(n == REFTABLE_API_ERROR); diff --git a/reftable/merged_test.c b/reftable/merged_test.c index fd5a065e42..6d1159d12d 100644 --- a/reftable/merged_test.c +++ b/reftable/merged_test.c @@ -123,15 +123,14 @@ static void readers_destroy(struct reftable_reader **readers, size_t n) static void test_merged_between(void) { - char a[] = "a", b[] = "b"; struct reftable_ref_record r1[] = { { - .refname = b, + .refname = (char *) "b", .update_index = 1, .value_type = REFTABLE_REF_VAL1, .value.val1 = { 1, 2, 3, 0 }, } }; struct reftable_ref_record r2[] = { { - .refname = a, + .refname = (char *) "a", .update_index = 2, .value_type = REFTABLE_REF_DELETION, } }; @@ -164,41 +163,40 @@ static void test_merged_between(void) static void test_merged(void) { - char a[] = "a", b[] = "b", c[] = "c", d[] = "d"; struct reftable_ref_record r1[] = { { - .refname = a, + .refname = (char *) "a", .update_index = 1, .value_type = REFTABLE_REF_VAL1, .value.val1 = { 1 }, }, { - .refname = b, + .refname = (char *) "b", .update_index = 1, .value_type = REFTABLE_REF_VAL1, .value.val1 = { 1 }, }, { - .refname = c, + .refname = (char *) "c", .update_index = 1, .value_type = REFTABLE_REF_VAL1, .value.val1 = { 1 }, } }; struct reftable_ref_record r2[] = { { - .refname = a, + .refname = (char *) "a", .update_index = 2, .value_type = REFTABLE_REF_DELETION, } }; struct reftable_ref_record r3[] = { { - .refname = c, + .refname = (char *) "c", .update_index = 3, .value_type = REFTABLE_REF_VAL1, .value.val1 = { 2 }, }, { - .refname = d, + .refname = (char *) "d", .update_index = 3, .value_type = REFTABLE_REF_VAL1, .value.val1 = { 1 }, @@ -291,52 +289,48 @@ merged_table_from_log_records(struct reftable_log_record **logs, static void test_merged_logs(void) { - char a[] = "a"; - char name[] = "jane doe", email[] = "jane@invalid"; - char message1[] = "message1", message2[] = "message2"; - char message3[] = "message3"; struct reftable_log_record r1[] = { { - .refname = a, + .refname = (char *) "a", .update_index = 2, .value_type = REFTABLE_LOG_UPDATE, .value.update = { .old_hash = { 2 }, /* deletion */ - .name = name, - .email = email, - .message = message2, + .name = (char *) "jane doe", + .email = (char *) "jane@invalid", + .message = (char *) "message2", } }, { - .refname = a, + .refname = (char *) "a", .update_index = 1, .value_type = REFTABLE_LOG_UPDATE, .value.update = { .old_hash = { 1 }, .new_hash = { 2 }, - .name = name, - .email = email, - .message = message1, + .name = (char *) "jane doe", + .email = (char *) "jane@invalid", + .message = (char *) "message1", } }, }; struct reftable_log_record r2[] = { { - .refname = a, + .refname = (char *) "a", .update_index = 3, .value_type = REFTABLE_LOG_UPDATE, .value.update = { .new_hash = { 3 }, - .name = name, - .email = email, - .message = message3, + .name = (char *) "jane doe", + .email = (char *) "jane@invalid", + .message = (char *) "message3", } }, }; struct reftable_log_record r3[] = { { - .refname = a, + .refname = (char *) "a", .update_index = 2, .value_type = REFTABLE_LOG_DELETION, }, @@ -406,13 +400,13 @@ static void test_merged_logs(void) static void test_default_write_opts(void) { - char master[] = "master"; struct reftable_write_options opts = { 0 }; struct strbuf buf = STRBUF_INIT; struct reftable_writer *w = reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts); + struct reftable_ref_record rec = { - .refname = master, + .refname = (char *) "master", .update_index = 1, }; int err; diff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c index 064d693111..c55019232b 100644 --- a/reftable/readwrite_test.c +++ b/reftable/readwrite_test.c @@ -56,7 +56,6 @@ static void write_table(char ***names, struct strbuf *buf, int N, int i = 0, n; struct reftable_log_record log = { NULL }; const struct reftable_stats *stats = NULL; - char message[] = "message"; REFTABLE_CALLOC_ARRAY(*names, N + 1); @@ -87,7 +86,7 @@ static void write_table(char ***names, struct strbuf *buf, int N, log.update_index = update_index; log.value_type = REFTABLE_LOG_UPDATE; set_test_hash(log.value.update.new_hash, i); - log.value.update.message = message; + log.value.update.message = (char *) "message"; n = reftable_writer_add_log(w, &log); EXPECT(n == 0); @@ -112,28 +111,23 @@ static void write_table(char ***names, struct strbuf *buf, int N, static void test_log_buffer_size(void) { - char refname[] = "refs/heads/master"; - char name[] = "Han-Wen Hienhuys"; - char email[] = "hanwen@google.com"; - char message[] = "commit: 9\n"; struct strbuf buf = STRBUF_INIT; struct reftable_write_options opts = { .block_size = 4096, }; int err; int i; - struct reftable_log_record log = { - .refname = refname, - .update_index = 0xa, - .value_type = REFTABLE_LOG_UPDATE, - .value.update = { - .name = name, - .email = email, - .tz_offset = 100, - .time = 0x5e430672, - .message = message, - }, - }; + struct reftable_log_record + log = { .refname = (char *) "refs/heads/master", + .update_index = 0xa, + .value_type = REFTABLE_LOG_UPDATE, + .value = { .update = { + .name = (char *) "Han-Wen Nienhuys", + .email = (char *) "hanwen@google.com", + .tz_offset = 100, + .time = 0x5e430672, + .message = (char *) "commit: 9\n", + } } }; struct reftable_writer *w = reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts); @@ -155,9 +149,6 @@ static void test_log_buffer_size(void) static void test_log_overflow(void) { - char refname[] = "refs/heads/master"; - char name[] = "Han-Wen Hienhuys"; - char email[] = "hanwen@google.com"; struct strbuf buf = STRBUF_INIT; char msg[256] = { 0 }; struct reftable_write_options opts = { @@ -165,15 +156,15 @@ static void test_log_overflow(void) }; int err; struct reftable_log_record log = { - .refname = refname, + .refname = (char *) "refs/heads/master", .update_index = 0xa, .value_type = REFTABLE_LOG_UPDATE, .value = { .update = { .old_hash = { 1 }, .new_hash = { 2 }, - .name = name, - .email = email, + .name = (char *) "Han-Wen Nienhuys", + .email = (char *) "hanwen@google.com", .tz_offset = 100, .time = 0x5e430672, .message = msg, @@ -299,20 +290,17 @@ static void test_log_zlib_corruption(void) struct reftable_writer *w = reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts); const struct reftable_stats *stats = NULL; - char refname[] = "refname"; - char name[] = "My Name"; - char email[] = "myname@invalid"; char message[100] = { 0 }; int err, i, n; struct reftable_log_record log = { - .refname = refname, + .refname = (char *) "refname", .value_type = REFTABLE_LOG_UPDATE, .value = { .update = { .new_hash = { 1 }, .old_hash = { 2 }, - .name = name, - .email = email, + .name = (char *) "My Name", + .email = (char *) "myname@invalid", .message = message, }, }, @@ -739,9 +727,8 @@ static void test_write_empty_key(void) struct strbuf buf = STRBUF_INIT; struct reftable_writer *w = reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts); - char refname[] = ""; struct reftable_ref_record ref = { - .refname = refname, + .refname = (char *) "", .update_index = 1, .value_type = REFTABLE_REF_DELETION, }; @@ -763,21 +750,20 @@ static void test_write_key_order(void) struct strbuf buf = STRBUF_INIT; struct reftable_writer *w = reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts); - char a[] = "a", b[] = "b", target[] = "target"; struct reftable_ref_record refs[2] = { { - .refname = b, + .refname = (char *) "b", .update_index = 1, .value_type = REFTABLE_REF_SYMREF, .value = { - .symref = target, + .symref = (char *) "target", }, }, { - .refname = a, + .refname = (char *) "a", .update_index = 1, .value_type = REFTABLE_REF_SYMREF, .value = { - .symref = target, + .symref = (char *) "target", }, } }; diff --git a/reftable/stack.c b/reftable/stack.c index a59ebe038d..09549c51c9 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -204,7 +204,8 @@ static struct reftable_reader **stack_copy_readers(struct reftable_stack *st, return cur; } -static int reftable_stack_reload_once(struct reftable_stack *st, char **names, +static int reftable_stack_reload_once(struct reftable_stack *st, + const char **names, int reuse_open) { size_t cur_len = !st->merged ? 0 : st->merged->stack_len; @@ -222,7 +223,7 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names, while (*names) { struct reftable_reader *rd = NULL; - char *name = *names++; + const char *name = *names++; /* this is linear; we assume compaction keeps the number of tables under control so this is not quadratic. */ @@ -354,7 +355,7 @@ static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st, goto out; } - err = reftable_stack_reload_once(st, names, reuse_open); + err = reftable_stack_reload_once(st, (const char **) names, reuse_open); if (!err) break; if (err != REFTABLE_NOT_EXIST_ERROR) @@ -368,7 +369,8 @@ static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st, err = read_lines(st->list_file, &names_after); if (err < 0) goto out; - if (names_equal(names_after, names)) { + if (names_equal((const char **) names_after, + (const char **) names)) { err = REFTABLE_NOT_EXIST_ERROR; goto out; } diff --git a/reftable/stack_test.c b/reftable/stack_test.c index c6d88e6ea8..4abf92636d 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -116,14 +116,13 @@ static void test_parse_names(void) static void test_names_equal(void) { - char a[] = "a", b[] = "b", c[] = "c", d[] = "d"; - char *abc[] = { a, b, c, NULL }; - char *abd[] = { a, b, d, NULL }; - char *ab[] = { a, b, NULL }; - - EXPECT(names_equal(abc, abc)); - EXPECT(!names_equal(abc, abd)); - EXPECT(!names_equal(abc, ab)); + const char *a[] = { "a", "b", "c", NULL }; + const char *b[] = { "a", "b", "d", NULL }; + const char *c[] = { "a", "b", NULL }; + + EXPECT(names_equal(a, a)); + EXPECT(!names_equal(a, b)); + EXPECT(!names_equal(a, c)); } static int write_test_ref(struct reftable_writer *wr, void *arg) @@ -156,12 +155,11 @@ static void test_reftable_stack_add_one(void) }; struct reftable_stack *st = NULL; int err; - char head[] = "HEAD", master[] = "master"; struct reftable_ref_record ref = { - .refname = head, + .refname = (char *) "HEAD", .update_index = 1, .value_type = REFTABLE_REF_SYMREF, - .value.symref = master, + .value.symref = (char *) "master", }; struct reftable_ref_record dest = { NULL }; struct stat stat_result = { 0 }; @@ -217,18 +215,17 @@ static void test_reftable_stack_uptodate(void) char *dir = get_tmp_dir(__LINE__); int err; - char head[] = "HEAD", branch2[] = "branch2", master[] = "master"; struct reftable_ref_record ref1 = { - .refname = head, + .refname = (char *) "HEAD", .update_index = 1, .value_type = REFTABLE_REF_SYMREF, - .value.symref = master, + .value.symref = (char *) "master", }; struct reftable_ref_record ref2 = { - .refname = branch2, + .refname = (char *) "branch2", .update_index = 2, .value_type = REFTABLE_REF_SYMREF, - .value.symref = master, + .value.symref = (char *) "master", }; @@ -265,12 +262,12 @@ static void test_reftable_stack_transaction_api(void) struct reftable_stack *st = NULL; int err; struct reftable_addition *add = NULL; - char head[] = "HEAD", master[] = "master"; + struct reftable_ref_record ref = { - .refname = head, + .refname = (char *) "HEAD", .update_index = 1, .value_type = REFTABLE_REF_SYMREF, - .value.symref = master, + .value.symref = (char *) "master", }; struct reftable_ref_record dest = { NULL }; @@ -313,11 +310,10 @@ static void test_reftable_stack_transaction_api_performs_auto_compaction(void) EXPECT_ERR(err); for (i = 0; i <= n; i++) { - char master[] = "master"; struct reftable_ref_record ref = { .update_index = reftable_stack_next_update_index(st), .value_type = REFTABLE_REF_SYMREF, - .value.symref = master, + .value.symref = (char *) "master", }; char name[100]; @@ -359,9 +355,8 @@ static void test_reftable_stack_transaction_api_performs_auto_compaction(void) static void test_reftable_stack_auto_compaction_fails_gracefully(void) { - char master[] = "refs/meads/master"; struct reftable_ref_record ref = { - .refname = master, + .refname = (char *) "refs/heads/master", .update_index = 1, .value_type = REFTABLE_REF_VAL1, .value.val1 = {0x01}, @@ -409,21 +404,21 @@ static int write_error(struct reftable_writer *wr, void *arg) static void test_reftable_stack_update_index_check(void) { char *dir = get_tmp_dir(__LINE__); + struct reftable_write_options cfg = { 0 }; struct reftable_stack *st = NULL; int err; - char name1[] = "name1", name2[] = "name2", master[] = "master"; struct reftable_ref_record ref1 = { - .refname = name1, + .refname = (char *) "name1", .update_index = 1, .value_type = REFTABLE_REF_SYMREF, - .value.symref = master, + .value.symref = (char *) "master", }; struct reftable_ref_record ref2 = { - .refname = name2, + .refname = (char *) "name2", .update_index = 1, .value_type = REFTABLE_REF_SYMREF, - .value.symref = master, + .value.symref = (char *) "master", }; err = reftable_new_stack(&st, dir, cfg); @@ -565,12 +560,8 @@ static void test_reftable_stack_log_normalize(void) }; struct reftable_stack *st = NULL; char *dir = get_tmp_dir(__LINE__); - char branch[] = "branch"; - char onetwomessage[] = "one\ntwo"; - char onemessage[] = "one"; - char twomessage[] = "two\n"; struct reftable_log_record input = { - .refname = branch, + .refname = (char *) "branch", .update_index = 1, .value_type = REFTABLE_LOG_UPDATE, .value = { @@ -591,11 +582,11 @@ static void test_reftable_stack_log_normalize(void) err = reftable_new_stack(&st, dir, cfg); EXPECT_ERR(err); - input.value.update.message = onetwomessage; + input.value.update.message = (char *) "one\ntwo"; err = reftable_stack_add(st, &write_test_log, &arg); EXPECT(err == REFTABLE_API_ERROR); - input.value.update.message = onemessage; + input.value.update.message = (char *) "one"; err = reftable_stack_add(st, &write_test_log, &arg); EXPECT_ERR(err); @@ -603,7 +594,7 @@ static void test_reftable_stack_log_normalize(void) EXPECT_ERR(err); EXPECT(0 == strcmp(dest.value.update.message, "one\n")); - input.value.update.message = twomessage; + input.value.update.message = (char *) "two\n"; arg.update_index = 2; err = reftable_stack_add(st, &write_test_log, &arg); EXPECT_ERR(err); @@ -700,14 +691,15 @@ static void test_reftable_stack_tombstone(void) static void test_reftable_stack_hash_id(void) { char *dir = get_tmp_dir(__LINE__); + struct reftable_write_options cfg = { 0 }; struct reftable_stack *st = NULL; int err; - char master[] = "master", target[] = "target"; + struct reftable_ref_record ref = { - .refname = master, + .refname = (char *) "master", .value_type = REFTABLE_REF_SYMREF, - .value.symref = target, + .value.symref = (char *) "target", .update_index = 1, }; struct reftable_write_options cfg32 = { .hash_id = GIT_SHA256_FORMAT_ID }; @@ -882,12 +874,12 @@ static void test_reftable_stack_auto_compaction(void) EXPECT_ERR(err); for (i = 0; i < N; i++) { - char name[100], master[] = "master"; + char name[100]; struct reftable_ref_record ref = { .refname = name, .update_index = reftable_stack_next_update_index(st), .value_type = REFTABLE_REF_SYMREF, - .value.symref = master, + .value.symref = (char *) "master", }; snprintf(name, sizeof(name), "branch%04d", i); @@ -918,11 +910,10 @@ static void test_reftable_stack_add_performs_auto_compaction(void) EXPECT_ERR(err); for (i = 0; i <= n; i++) { - char master[] = "master"; struct reftable_ref_record ref = { .update_index = reftable_stack_next_update_index(st), .value_type = REFTABLE_REF_SYMREF, - .value.symref = master, + .value.symref = (char *) "master", }; /* @@ -968,12 +959,12 @@ static void test_reftable_stack_compaction_concurrent(void) EXPECT_ERR(err); for (i = 0; i < N; i++) { - char name[100], master[] = "master"; + char name[100]; struct reftable_ref_record ref = { .refname = name, .update_index = reftable_stack_next_update_index(st1), .value_type = REFTABLE_REF_SYMREF, - .value.symref = master, + .value.symref = (char *) "master", }; snprintf(name, sizeof(name), "branch%04d", i); @@ -1018,12 +1009,12 @@ static void test_reftable_stack_compaction_concurrent_clean(void) EXPECT_ERR(err); for (i = 0; i < N; i++) { - char name[100], master[] = "master"; + char name[100]; struct reftable_ref_record ref = { .refname = name, .update_index = reftable_stack_next_update_index(st1), .value_type = REFTABLE_REF_SYMREF, - .value.symref = master, + .value.symref = (char *) "master", }; snprintf(name, sizeof(name), "branch%04d", i);
-- 2.45.1.410.g58bac47f8e.dirty