On 14/12/2022 15:35, Rose via GitGitGadget wrote:
From: Seija Kijin <doremylover123@xxxxxxxxx>
Many of these casts remain,
even though the target variable is the type it is being casted to.
We can safely remove said casts.
Like Peff I curious how you found these. I also agree that it would be
helpful to understand the history so we can be sure there the cast is
not a sympton of a bug - Peff has given you a great start on that. A lot
of the changes look like useful cleanups but there are a couple that
caught my eye as being wrong or undesirable which I've commented on below.
diff --git a/strbuf.c b/strbuf.c
index 0890b1405c5..940f59473eb 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -493,7 +493,7 @@ void strbuf_add_percentencode(struct strbuf *dst, const char *src, int flags)
if (ch <= 0x1F || ch >= 0x7F ||
(ch == '/' && (flags & STRBUF_ENCODE_SLASH)) ||
strchr(URL_UNSAFE_CHARS, ch))
- strbuf_addf(dst, "%%%02X", (unsigned char)ch);
+ strbuf_addf(dst, "%%%02X", ch);
This one is wrong, the cast is required if char is signed as in that
case it will be converted to a signed integer (because printf() takes a
variable number of arguments) and then %X treats that integer as
unsigned. That means that if the high bit is set we'll now print a bunch
of leading F's. To see this compile and run
#include <stdio.h>
int main(void)
{
printf("%02X %02X\n", (signed char)0x80, (unsigned char)0x80);
return 0;
}
which prints
FFFFFF80 80
diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
index c84549f6c50..04fa4e5a01d 100644
--- a/xdiff/xprepare.c
+++ b/xdiff/xprepare.c
@@ -188,7 +188,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_
if (!(crec = xdl_cha_alloc(&xdf->rcha)))
goto abort;
crec->ptr = prev;
- crec->size = (long) (cur - prev);
+ crec->size = (cur - prev);
I'm not sure if we want to remove this cast. cur and prev are pointers
so (cur - prev) has type ptrdiff_t which maybe wider than long. We can
debate whether we should be using a cast to hide any compiler warning
here but the cast is not redundant in the same way as others in this patch.
crec->ha = hav;
recs[nrec++] = crec;
if (xdl_classify_record(pass, cf, rhash, hbits, crec) < 0)
diff --git a/xdiff/xutils.c b/xdiff/xutils.c
index 9e36f24875d..853f2260a1d 100644
--- a/xdiff/xutils.c
+++ b/xdiff/xutils.c
@@ -130,7 +130,7 @@ long xdl_guess_lines(mmfile_t *mf, long sample) {
else
cur++;
}
- tsize += (long) (cur - data);
+ tsize += (cur - data);
This is in the same class as the one above. (Also if we do end up
removing the cast we should remove the parentheses as well)
Best Wishes
Phillip
