Download raw body.
build with -Wwrite-strings
to complement yesterday's diff about -Wmissing-prototypes, here's an attempt to build got with -Wwrite-strings. The purpose is to enforce some `const's here and there, in the hope to make some parts clearer. It correctly picked up a few places where we were using "char *" (or "void *") but passing "const" pointers, and I'd also argue that in the case of got_diff_objects_as_trees it's actually better to have the pointers correctly marked as const. There are also two downsides however: - i need to add a cast in got_dial_ssh. I don't know how to trackle argv without extra casts with -Wwrite-strings. The compiler complained that assigning to argv[x] a const char * was loosing const, but execv wants a "char *const *", and I can't have argv declared as `char *const argv[11]' either. (why execv has such signature? meh) - got_path_find_prog becomes more complex. it mixes const and non-const char pointers, but it's carefully designed to not modify the const strings. The compiler isn't smart enough to notice it (nor that it should.) A possible solution in the long-term could be to maybe drop this function: it's just used to find the editor to exec; we could just try to exec $VISUAL, $EDITOR and /bin/ed in cascade and use the first that works. not for today tho :) so i'm not sure if we want this in or to just backport some changes and call it a day. P.S.: tracey: I'm probably making your life in the gotwebd branch harder by adding these warnings (that turns into errors thanks to -Werror in non release builds.) I'll try to see if gotwebd breaks with these warnings enabled and cook a diff soon(tm) ;) ----------------------------------------------- commit c02440c56013c8818bf510752cfed899a4c8abd6 (wwritestr) from: Omar Polo <op@omarpolo.com> date: Sun Jun 26 09:01:05 2022 UTC build with -Wwrite-strings diff 3d589bee0bbbe812bb91f3b0284fbf2596304132 c02440c56013c8818bf510752cfed899a4c8abd6 commit - 3d589bee0bbbe812bb91f3b0284fbf2596304132 commit + c02440c56013c8818bf510752cfed899a4c8abd6 blob - ef99224d337170364b2e5458bf6e2a3206c3e05a blob + 2b72df1a3c9e149cff31460ae7090698a3416a61 --- Makefile.inc +++ Makefile.inc @@ -11,7 +11,7 @@ LIBEXECDIR ?= ${PREFIX}/libexec MANDIR ?= ${PREFIX}/man/man .else CFLAGS += -Werror -Wall -Wstrict-prototypes -Wmissing-prototypes -CFLAGS += -Wunused-variable +CFLAGS += -Wwrite-strings -Wunused-variable PREFIX ?= ${HOME} BINDIR ?= ${PREFIX}/bin LIBEXECDIR ?= ${BINDIR} blob - e1fc7e795e917a7be22aff253610e8c66eef455c blob + a6bd0096cd2d6e29e21d56f19ed44cd2365eb1ba --- gotweb/gotweb.c +++ gotweb/gotweb.c @@ -237,7 +237,7 @@ struct gw_query_action { unsigned int func_id; const char *func_name; const struct got_error *(*func_main)(struct gw_trans *); - char *template; + const char *template; }; enum gw_query_actions { @@ -2716,11 +2716,11 @@ gw_get_time_str(char **repo_age, time_t committer_time { struct tm tm; time_t diff_time; - char *years = "years ago", *months = "months ago"; - char *weeks = "weeks ago", *days = "days ago", *hours = "hours ago"; - char *minutes = "minutes ago", *seconds = "seconds ago"; - char *now = "right now"; - char *s; + const char *years = "years ago", *months = "months ago"; + const char *weeks = "weeks ago", *days = "days ago", *hours = "hours ago"; + const char *minutes = "minutes ago", *seconds = "seconds ago"; + const char *now = "right now"; + const char *s; char datebuf[29]; *repo_age = NULL; @@ -4763,7 +4763,7 @@ static const struct got_error * gw_colordiff_line(struct gw_trans *gw_trans, char *buf) { const struct got_error *error = NULL; - char *color = NULL; + const char *color = NULL; enum kcgi_err kerr = KCGI_OK; if (strncmp(buf, "-", 1) == 0) blob - 5dbe18b856a53a58bee722d773249a93e91993a8 blob + 4f537113e196997bfae7b568a9d90ab8fe9e77b0 --- include/got_diff.h +++ include/got_diff.h @@ -166,7 +166,7 @@ const struct got_error *got_diff_objects_as_blobs(off_ */ const struct got_error *got_diff_objects_as_trees(off_t **, size_t *, FILE *, FILE *, struct got_object_id *, struct got_object_id *, - struct got_pathlist_head *, char *, char *, int, int, int, + struct got_pathlist_head *, const char *, const char *, int, int, int, struct got_repository *, FILE *); /* blob - 4f688a96c44c72c60f3157e3d1293fcca25111c6 blob + b7ce464748cabe3f0823af08f8961d87d13e0c7a --- lib/dial.c +++ lib/dial.c @@ -67,7 +67,7 @@ got_dial_apply_unveil(const char *proto) } static int -hassuffix(char *base, char *suf) +hassuffix(const char *base, const char *suf) { int nb, ns; @@ -207,7 +207,7 @@ got_dial_ssh(pid_t *newpid, int *newfd, const char *ho const struct got_error *error = NULL; int pid, pfd[2]; char cmd[64]; - char *argv[11]; + const char *argv[11]; int i = 0, j; *newpid = -1; @@ -252,7 +252,7 @@ got_dial_ssh(pid_t *newpid, int *newfd, const char *ho n = snprintf(cmd, sizeof(cmd), "git-%s-pack", direction); if (n < 0 || n >= ssizeof(cmd)) err(1, "snprintf"); - if (execv(GOT_DIAL_PATH_SSH, argv) == -1) + if (execv(GOT_DIAL_PATH_SSH, (char *const *)argv) == -1) err(1, "execv"); abort(); /* not reached */ } else { blob - 8337d25f3e5608a25af5b7e4a9940d9bcb2ceba5 blob + 773169c32d3cb119ac330201afe43d53b115085a --- lib/diff.c +++ lib/diff.c @@ -62,7 +62,7 @@ diff_blobs(off_t **line_offsets, size_t *nlines, const struct got_error *err = NULL, *free_err; char hex1[SHA1_DIGEST_STRING_LENGTH]; char hex2[SHA1_DIGEST_STRING_LENGTH]; - char *idstr1 = NULL, *idstr2 = NULL; + const char *idstr1 = NULL, *idstr2 = NULL; off_t size1, size2; struct got_diffreg_result *result; off_t outoff = 0; @@ -220,7 +220,7 @@ diff_blob_file(struct got_diffreg_result **resultp, { const struct got_error *err = NULL, *free_err; char hex1[SHA1_DIGEST_STRING_LENGTH]; - char *idstr1 = NULL; + const char *idstr1 = NULL; struct got_diffreg_result *result = NULL; if (resultp) @@ -904,9 +904,9 @@ show_object_id(off_t **line_offsets, size_t *nlines, c static const struct got_error * diff_objects_as_trees(off_t **line_offsets, size_t *nlines, FILE *f1, FILE *f2, struct got_object_id *id1, struct got_object_id *id2, - struct got_pathlist_head *paths, - char *label1, char *label2, int diff_context, int ignore_whitespace, - int force_text_diff, struct got_repository *repo, FILE *outfile) + struct got_pathlist_head *paths, const char *label1, const char *label2, + int diff_context, int ignore_whitespace, int force_text_diff, + struct got_repository *repo, FILE *outfile) { const struct got_error *err; struct got_tree_object *tree1 = NULL, *tree2 = NULL; @@ -960,9 +960,9 @@ done: const struct got_error * got_diff_objects_as_trees(off_t **line_offsets, size_t *nlines, FILE *f1, FILE *f2, struct got_object_id *id1, struct got_object_id *id2, - struct got_pathlist_head *paths, - char *label1, char *label2, int diff_context, int ignore_whitespace, - int force_text_diff, struct got_repository *repo, FILE *outfile) + struct got_pathlist_head *paths, const char *label1, const char *label2, + int diff_context, int ignore_whitespace, int force_text_diff, + struct got_repository *repo, FILE *outfile) { const struct got_error *err; char *idstr = NULL; blob - 70383527274d9e8c501d2480b0479657ced378b7 blob + e44ac0a4e3f7fe9caf5b350a89c7e5c5beba00be --- lib/diff_output.c +++ lib/diff_output.c @@ -262,7 +262,7 @@ diff_output_match_function_prototype(char *prototype, struct diff_atom *start_atom, *atom; const struct diff_data *data; unsigned char buf[DIFF_FUNCTION_CONTEXT_SIZE]; - char *state = NULL; + const char *state = NULL; int rc, i, ch; if (result->left->atoms.len > 0 && cc->left.start > 0) { blob - 16d1a758fc03fec245de789888abc9cb072f408d blob + 384b6d4beb4ecb887c5ba512e0dd1f882bb02d4b --- lib/pack_create.c +++ lib/pack_create.c @@ -1829,7 +1829,7 @@ done: } static const struct got_error * -hwrite(FILE *f, void *buf, off_t len, SHA1_CTX *ctx) +hwrite(FILE *f, const void *buf, off_t len, SHA1_CTX *ctx) { size_t n; blob - 19dc9c983445c68dbea181cf6bfabcfd1ee9d949 blob + e3d7799acfdd15ffc55d19126b7c7e27a696bee6 --- lib/path.c +++ lib/path.c @@ -458,10 +458,11 @@ const struct got_error * got_path_find_prog(char **filename, const char *prog) { const struct got_error *err = NULL; + const char *path; char *p; int len; struct stat sbuf; - char *path, *pathcpy; + char *pathcpy, *dup = NULL; *filename = NULL; @@ -480,19 +481,22 @@ got_path_find_prog(char **filename, const char *prog) return NULL; } - if ((path = strdup(path)) == NULL) + if ((dup = strdup(path)) == NULL) return got_error_from_errno("strdup"); - pathcpy = path; + pathcpy = dup; while ((p = strsep(&pathcpy, ":")) != NULL) { - if (*p == '\0') - p = "."; + const char *d; len = strlen(p); while (len > 0 && p[len-1] == '/') p[--len] = '\0'; /* strip trailing '/' */ - if (asprintf(filename, "%s/%s", p, prog) == -1) { + d = p; + if (*d == '\0') + d = "."; + + if (asprintf(filename, "%s/%s", d, prog) == -1) { err = got_error_from_errno("asprintf"); break; } @@ -503,7 +507,7 @@ got_path_find_prog(char **filename, const char *prog) *filename = NULL; continue; } - free(path); + free(dup); return err; } blob - 8616b4e5f3930bdff53402d97880b3ae74a6fa5f blob + de71b5845e334bff8e56366f910514f149cf3519 --- lib/reference.c +++ lib/reference.c @@ -998,8 +998,8 @@ got_ref_list(struct got_reflist_head *refs, struct got { const struct got_error *err; char *packed_refs_path, *path_refs = NULL; - char *abs_namespace = NULL; - char *buf = NULL, *ondisk_ref_namespace = NULL; + char *abs_namespace = NULL, *buf = NULL; + const char *ondisk_ref_namespace = NULL; char *line = NULL; FILE *f = NULL; struct got_reference *ref; blob - 9d84105f23ebcedee81341088dc0fd38cb0c4261 blob + c0e71877fd2d42e62b12140d90a427fecf8d0cee --- regress/fetch/fetch_test.c +++ regress/fetch/fetch_test.c @@ -46,7 +46,7 @@ static int verbose; static int quiet; static void -test_printf(char *fmt, ...) +test_printf(const char *fmt, ...) { va_list ap; blob - e8001a520fd4f5763bc38dbfb1c94be3fdd0e531 blob + bbf71a79eb4fa68027df6f06b7a1d9926000a518 --- regress/path/path_test.c +++ regress/path/path_test.c @@ -34,7 +34,7 @@ static int verbose; static int quiet; static void -test_printf(char *fmt, ...) +test_printf(const char *fmt, ...) { va_list ap;
build with -Wwrite-strings