Re: FYI, placating valgrind, wrt ./cp --sparse=always

[Jim Meyering]

Andreas Schwab <address@hidden> wrote:
> Jim Meyering <address@hidden> writes:
>
>> diff --git a/src/copy.c b/src/copy.c
>> index a3489c3..f0c6539 100644
>> --- a/src/copy.c
>> +++ b/src/copy.c
>> @@ -430,7 +430,8 @@ copy_reg (char const *src_name, char const *dst_name,
>>          {
>>            char *cp;
>>
>> -          buf[n_read] = 1;  /* Sentinel to stop loop.  */
>> +          wp = (word *) (buf + n_read);
>> +          *wp = 1;  /* Sentinel to stop loop.  */
>
> What if n_read is uneven?

Oops.  Or even, but not a multiple of sizeof(word).
Unaligned access is bad.

I've done this instead:

        Adjust preceding change not to perform an unaligned access.
        * src/copy.c (copy_reg): Undo previous change.  Instead, make
        it clearer that we're using a single-byte sentinel, and
        [lint]: Initialize uintptr_t-1 bytes after the sentinel.
        Reported by Andreas Schwab.

Thanks!

diff --git a/src/copy.c b/src/copy.c
index f0c6539..99e2ca4 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -430,8 +430,17 @@ copy_reg (char const *src_name, char const *dst_name,
            {
              char *cp;

-             wp = (word *) (buf + n_read);
-             *wp = 1;  /* Sentinel to stop loop.  */
+             /* Sentinel to stop loop.  */
+             buf[n_read] = '\1';
+#ifdef lint
+             /* Usually, buf[n_read] is not the byte just before a "word"
+                (aka uintptr_t) boundary.  In that case, the word-oriented
+                test below (*wp++ == 0) would read some uninitialized bytes
+                after the sentinel.  To avoid false-positive reports about
+                this condition (e.g., from a tool like valgrind), set the
+                remaining bytes -- to any value.  */
+             memset (buf + n_read + 1, 0, sizeof (word) - 1);
+#endif

              /* Find first nonzero *word*, or the word with the sentinel.  */