[PATCH v2] Cygwin: clipboard: Fix a bug in read().
Wed Dec 8 05:53:58 GMT 2021
On 2021-12-07 13:18, Thomas Wolff wrote:
> Am 07.12.2021 um 15:23 schrieb Corinna Vinschen:
>> On Dec 7 23:00, Takashi Yano wrote:
>>> - Fix a bug in fhandler_dev_clipboard::read() that the second read
>>> fails with 'Bad address'.
>>> winsup/cygwin/fhandler_clipboard.cc | 2 +-
>>> winsup/cygwin/release/3.3.4 | 6 ++++++
>>> 2 files changed, 7 insertions(+), 1 deletion(-)
>>> create mode 100644 winsup/cygwin/release/3.3.4
>>> diff --git a/winsup/cygwin/fhandler_clipboard.cc
>>> index 0b87dd352..ae10228a7 100644
>>> --- a/winsup/cygwin/fhandler_clipboard.cc
>>> +++ b/winsup/cygwin/fhandler_clipboard.cc
>>> @@ -229,7 +229,7 @@ fhandler_dev_clipboard::read (void *ptr, size_t&
>>> if (pos < (off_t) clipbuf->cb_size)
>>> ret = (len > (clipbuf->cb_size - pos)) ? clipbuf->cb_size -
>>> pos : len;
>>> - memcpy (ptr, &clipbuf + pos , ret);
>>> + memcpy (ptr, (char *) &clipbuf + pos, ret);
>> I'm always cringing a bit when I see this kind of expression. Personally
>> I think (ptr + offset) is easier to read than &ptr[offset], but of course
>> that's just me. If you agree, would it be ok to change the above to
>> (char *) (clipbuf + 1)
>> while you're at it? If you like the ampersand expression more, it's ok,
>> too, of course. Please push.
> In this specific case I think it's actually more confusing because of
> the type mangling that's intended in the clipbuf.
> At quick glance, it looks a bit as if the following were meant:
> (char *) clipbuf + 1
> I'd even make it clearer like
> + memcpy (ptr, ((char *) &clipbuf) + pos, ret);
> or even
> + memcpy (ptr, ((char *) (&clipbuf)) + pos, ret);
If the intent is to address:
clipbuf + pos + 1
use either that or:
&clipbuf[pos + 1]
to avoid obscuring the intent,
and add comments to make it clearer!
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada
This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
[Data in binary units and prefixes, physical quantities in SI.]
More information about the Cygwin-patches