Re: xfer_sum_len type bug
On Fri, May 26, 2023 at 08:34:20AM -0700, Wayne Davison wrote: > On Tue, May 16, 2023 at 2:03 PM Derek Martin wrote: > > > This appears to be because of a type mismatch between xfer_sum_len > > (declared as [signed] int) and the third arugment to memset, whose > > function prototype is (from the man page): > > > >void *memset(void *s, int c, size_t n); > > > > If that is the case, why does it not complain about file_sum_len, which is > also an int that is passed to memset()? The only difference I see is that > xfer_sum_len's memset() is in match.c while file_sum_len's memset() is in > checksum.c, and the only difference I can see with that is that the int is > local to checksum.c and extern in match.c. Very strange. I'm more inclined > to change them into unsigned short values, as they are quite small integers. I can't answer that question--I'm no expert on GCC internals and what trips its paranoia checks. I wouldn't be surprised if it was due to a bug in GCC causing the checks to be overzealous--when I googled it appeared that there might have been such a bug. However it's hard to deny that the types are indeed mismatched, or that properly matching them will certainly eliminate the possibility of such problems going forward... As for unsigned short vs. size_t... You can certainly do what you want, but as far as I could tell the variable exists for the purpose of identifying the size in bytes of a thing, for which size_t explicitly exists, and it is only ever used in the context of things that are also size_t, so--regardless of the expected range of values--I don't see that it saves much or serves any useful purpose to avoid just making it size_t. -- Derek Martin Principal System Software Engineer Akamai Technologies demar...@akamai.com -- Please use reply-all for most replies to avoid omitting the mailing list. To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html
Re: xfer_sum_len type bug
On Tue, May 16, 2023 at 2:03 PM Derek Martin wrote: > This appears to be because of a type mismatch between xfer_sum_len > (declared as [signed] int) and the third arugment to memset, whose > function prototype is (from the man page): > >void *memset(void *s, int c, size_t n); > If that is the case, why does it not complain about file_sum_len, which is also an int that is passed to memset()? The only difference I see is that xfer_sum_len's memset() is in match.c while file_sum_len's memset() is in checksum.c, and the only difference I can see with that is that the int is local to checksum.c and extern in match.c. Very strange. I'm more inclined to change them into unsigned short values, as they are quite small integers. ..wayne.. -- Please use reply-all for most replies to avoid omitting the mailing list. To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html
xfer_sum_len type bug
Hi folks,
I have several build environments in which I must build rsync, and in
some, but not all of them, the build fails when built with -Wall
-Werror due to this warning:
In file included from /usr/include/string.h:495,
from
/home/demartin/BuildClients/cobrasync-8.80-alsi11-lib32/cobrasync/rsync-3.2.7/rsync.h:339,
from
/home/demartin/BuildClients/cobrasync-8.80-alsi11-lib32/cobrasync/rsync-3.2.7/match.c:22:
In function 'memset',
inlined from 'match_sums' at
/home/demartin/BuildClients/cobrasync-8.80-alsi11-lib32/cobrasync/rsync-3.2.7/match.c:431:3:
/usr/include/i386-linux-gnu/bits/string_fortified.h:71:10: error:
'__builtin___memset_chk' specified size between 2147483648 and 4294967295
exceeds maximum object size 2147483647 [-Werror=stringop-overflow=]
71 | return __builtin___memset_chk (__dest, __ch, __len, __bos0 (__dest));
| ^
cc1: all warnings being treated as errors
This appears to be because of a type mismatch between xfer_sum_len
(declared as [signed] int) and the third arugment to memset, whose
function prototype is (from the man page):
void *memset(void *s, int c, size_t n);
size_t != int.
I did check, and it appears to me that the other places xfer_sum_len
is used as a function argument expect a size_t. Assuming I haven't
missed any important details, the fix is simple enough (and does
indeed compile for me where the original does not):
-8K--
diff -Naur rsync-3.2.7.patch_orig/checksum.c rsync-3.2.7.patched/checksum.c
--- rsync-3.2.7.patch_orig/checksum.c 2023-05-16 14:17:31.078521657 -0400
+++ rsync-3.2.7.patched/checksum.c 2023-05-16 15:36:44.253892249 -0400
@@ -98,7 +98,7 @@
{ CSUM_MD5, NNI_BUILTIN, "md5", NULL };
struct name_num_item *xfer_sum_nni; /* used for the transfer checksum2
computations */
-int xfer_sum_len;
+size_t xfer_sum_len;
struct name_num_item *file_sum_nni; /* used for the pre-transfer --checksum
computations */
int file_sum_len, file_sum_extra_cnt;
diff -Naur rsync-3.2.7.patch_orig/match.c rsync-3.2.7.patched/match.c
--- rsync-3.2.7.patch_orig/match.c 2023-05-16 14:17:31.082521665 -0400
+++ rsync-3.2.7.patched/match.c 2023-05-16 15:36:46.781895867 -0400
@@ -32,7 +32,7 @@
extern int append_mode;
extern struct name_num_item *xfer_sum_nni;
-extern int xfer_sum_len;
+extern size_t xfer_sum_len;
int updating_basis_file;
char sender_file_sum[MAX_DIGEST_LEN];
diff -Naur rsync-3.2.7.patch_orig/receiver.c rsync-3.2.7.patched/receiver.c
--- rsync-3.2.7.patch_orig/receiver.c 2023-05-16 14:17:31.082521665 -0400
+++ rsync-3.2.7.patched/receiver.c 2023-05-16 15:37:10.401929552 -0400
@@ -75,7 +75,7 @@
extern OFF_T preallocated_len;
extern struct name_num_item *xfer_sum_nni;
-extern int xfer_sum_len;
+extern size_t xfer_sum_len;
static struct bitbag *delayed_bits = NULL;
static int phase = 0, redoing = 0;
-8K--
--
Derek Martin
Principal System Software Engineer
Akamai Technologies
demar...@akamai.com
--
Please use reply-all for most replies to avoid omitting the mailing list.
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html
