"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
Re: build with -Wwrite-strings
To:
Stefan Sperling <stsp@stsp.name>
Cc:
gameoftrees@openbsd.org
Date:
Sun, 26 Jun 2022 22:37:26 +0200

Download raw body.

Thread
Stefan Sperling <stsp@stsp.name> wrote:
> On Sun, Jun 26, 2022 at 11:16:51AM +0200, Omar Polo wrote:
> > 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.
> 
> No objection here. We could try it for a while and could get rid of it
> again if it becomes annoying. The workarounds you had to apply are not
> very bad, just awkward for someone who is not expecting const to be
> honoured strictly (it seems Wwrite-strings simply backports fancy C++11
> const semantics to plain C?)

no idea about the const semantics of C++, my knowledge there is very,
very limited.

alternatively i could add a few cast in got_path_find_prog to reduce the
difference with which' findprog

--- lib/path.c
+++ lib/path.c
@@ -467,7 +467,7 @@ got_path_find_prog(char **filename, const char *prog)
 
 	path = getenv("PATH");
 	if (path == NULL)
-		path = _PATH_DEFPATH;
+		path = (char *)_PATH_DEFPATH;
 
 	/* Special case if prog contains '/' */
 	if (strchr(prog, '/')) {
@@ -486,7 +486,7 @@ got_path_find_prog(char **filename, const char *prog)
 
 	while ((p = strsep(&pathcpy, ":")) != NULL) {
 		if (*p == '\0')
-			p = ".";
+			p = (char *)".";
 
 		len = strlen(p);
 		while (len > 0 && p[len-1] == '/')

but it's still ugly.

to add another info, a quick grep in /usr/src shows only file and tmux
to use -Wwrite-strings.