Re: PATCH: spelling correction buffer overflow

[Oliver Kiddle <okiddle@xxxxxxxxxxx>]

Kamil Dudka wrote:
> I spotted that this patch introduced new compiler warnings:
>
> Src/utils.c:4430:26: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
> #   return (new - newname) >= (sizeof(newname)-1) ? NULL : newname;

-Wsign-compare is not on by default and turning it on results in quite a
few warnings for the zsh code. It seems a little silly given that the
compiler ought to be able evaluate sizeof() at compile time and
establish that it is within the range of a signed integer.

> Should we cast RHS of the >= operator to ssize_t or ptrdiff_t to avoid them?

Casts are a bit ugly. The following - adding newname to both the LHS
and RHS - appears to avoid the warning by making both sides of the
comparison of type signed. At least until someone decides that adding an
unsigned to the signed should also generate a warning.

Oliver

diff --git a/Src/utils.c b/Src/utils.c
index eab407eee..3587c3622 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -4396,7 +4396,7 @@ spname(char *oldname)
      * Rationale for this, if there ever was any, has been forgotten.    */
     for (;;) {
 	while (*old == '/') {
-	    if ((new - newname) >= (sizeof(newname)-1))
+            if (new >= newname + sizeof(newname) - 1)
 		return NULL;
 	    *new++ = *old++;
 	}
@@ -4427,7 +4427,7 @@ spname(char *oldname)
 	    if (bestdist < maxthresh) {
 		struncpy(&new, spnameguess, sizeof(newname) - (new - newname));
 		struncpy(&new, old, sizeof(newname) - (new - newname));
-		return (new - newname) >= (sizeof(newname)-1) ? NULL : newname;
+		return (new >= newname + sizeof(newname) - 1) ? NULL : newname;
 	    } else
 	    	return NULL;
 	} else {
@@ -4435,7 +4435,7 @@ spname(char *oldname)
 	    bestdist += thisdist;
 	}
 	for (p = spnamebest; (*new = *p++);) {
-	    if ((new - newname) >= (sizeof(newname)-1))
+	    if (new >= newname + sizeof(newname) - 1)
 		return NULL;
 	    new++;
 	}