public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
* [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit
@ 2021-10-07  5:22 Mark Geisert
  2021-10-07  5:56 ` Mark Geisert
  2021-10-08  9:52 ` Takashi Yano
  0 siblings, 2 replies; 14+ messages in thread
From: Mark Geisert @ 2021-10-07  5:22 UTC (permalink / raw)
  To: cygwin-patches

This allows correct copy and pasting between the two Cygwin flavors.

What's done is to overlay the differing timespec formats via a union
within the control structure cygcb_t.  Then conversion between the two
formats is done on the 32-bit side only.

The cygutils package has two programs, putclip and getclip, that also
depend on the layout of the cygcb_t.  At present they have duplicate
defs of struct cygcb_t defined here as no Cygwin header provides it.

---
 winsup/cygwin/fhandler_clipboard.cc | 39 +++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/winsup/cygwin/fhandler_clipboard.cc b/winsup/cygwin/fhandler_clipboard.cc
index ccdb295f3..649e57072 100644
--- a/winsup/cygwin/fhandler_clipboard.cc
+++ b/winsup/cygwin/fhandler_clipboard.cc
@@ -28,9 +28,16 @@ static const WCHAR *CYGWIN_NATIVE = L"CYGWIN_NATIVE_CLIPBOARD";
 
 typedef struct
 {
-  timestruc_t	timestamp;
-  size_t	len;
-  char		data[1];
+  union
+  {
+    struct timespec ts; // 8 bytes on 32-bit Cygwin, 16 bytes on 64-bit Cygwin
+    struct
+    {
+      uint64_t	cb_sec;
+      uint64_t	cb_nsec;
+    };
+  };
+  uint64_t	cb_size;
 } cygcb_t;
 
 fhandler_dev_clipboard::fhandler_dev_clipboard ()
@@ -74,9 +81,14 @@ fhandler_dev_clipboard::set_clipboard (const void *buf, size_t len)
 	}
       clipbuf = (cygcb_t *) GlobalLock (hmem);
 
-      clock_gettime (CLOCK_REALTIME, &clipbuf->timestamp);
-      clipbuf->len = len;
-      memcpy (clipbuf->data, buf, len);
+      clock_gettime (CLOCK_REALTIME, &clipbuf->ts);
+#ifndef __x86_64__
+      // expand 32-bit timespec layout to 64-bit layout
+      clipbuf->cb_nsec = clipbuf->ts.tv_nsec;
+      clipbuf->cb_sec = clipbuf->ts.tv_sec;
+#endif
+      clipbuf->cb_size = len;
+      memcpy (&clipbuf[1], buf, len); // append data
 
       GlobalUnlock (hmem);
       EmptyClipboard ();
@@ -179,8 +191,13 @@ fhandler_dev_clipboard::fstat (struct stat *buf)
 	  && (hglb = GetClipboardData (format))
 	  && (clipbuf = (cygcb_t *) GlobalLock (hglb)))
 	{
-	  buf->st_atim = buf->st_mtim = clipbuf->timestamp;
-	  buf->st_size = clipbuf->len;
+#ifndef __x86_64__
+	  // compress 64-bit timespec layout to 32-bit layout
+	  clipbuf->ts.tv_sec = clipbuf->cb_sec;
+	  clipbuf->ts.tv_nsec = clipbuf->cb_nsec;
+#endif
+	  buf->st_atim = buf->st_mtim = clipbuf->ts;
+	  buf->st_size = clipbuf->cb_size;
 	  GlobalUnlock (hglb);
 	}
       CloseClipboard ();
@@ -218,10 +235,10 @@ fhandler_dev_clipboard::read (void *ptr, size_t& len)
     {
       cygcb_t *clipbuf = (cygcb_t *) cb_data;
 
-      if (pos < (off_t) clipbuf->len)
+      if (pos < (off_t) clipbuf->cb_size)
 	{
-	  ret = ((len > (clipbuf->len - pos)) ? (clipbuf->len - pos) : len);
-	  memcpy (ptr, clipbuf->data + pos , ret);
+	  ret = (len > (clipbuf->cb_size - pos)) ? clipbuf->cb_size - pos : len;
+	  memcpy (ptr, &clipbuf[1] + pos , ret);
 	  pos += ret;
 	}
     }
-- 
2.33.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit
  2021-10-07  5:22 [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit Mark Geisert
@ 2021-10-07  5:56 ` Mark Geisert
  2021-10-08  9:52 ` Takashi Yano
  1 sibling, 0 replies; 14+ messages in thread
From: Mark Geisert @ 2021-10-07  5:56 UTC (permalink / raw)
  To: cygwin-patches

Mark Geisert wrote:
> This allows correct copy and pasting between the two Cygwin flavors.
[...]

Eh, that's a bad patch. Don't apply. It's OK for concept review.
v2 forthcoming.
Thanks,

..mark

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit
  2021-10-07  5:22 [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit Mark Geisert
  2021-10-07  5:56 ` Mark Geisert
@ 2021-10-08  9:52 ` Takashi Yano
  2021-10-09 14:19   ` Ken Brown
  1 sibling, 1 reply; 14+ messages in thread
From: Takashi Yano @ 2021-10-08  9:52 UTC (permalink / raw)
  To: cygwin-patches

How about simply just:

diff --git a/winsup/cygwin/fhandler_clipboard.cc b/winsup/cygwin/fhandler_clipboard.cc
index ccdb295f3..d822f4fc4 100644
--- a/winsup/cygwin/fhandler_clipboard.cc
+++ b/winsup/cygwin/fhandler_clipboard.cc
@@ -28,9 +28,10 @@ static const WCHAR *CYGWIN_NATIVE = L"CYGWIN_NATIVE_CLIPBOARD";
 
 typedef struct
 {
-  timestruc_t	timestamp;
-  size_t	len;
-  char		data[1];
+  uint64_t tv_sec;
+  uint64_t tv_nsec;
+  uint64_t len;
+  char data[1];
 } cygcb_t;
 
 fhandler_dev_clipboard::fhandler_dev_clipboard ()
@@ -74,7 +75,10 @@ fhandler_dev_clipboard::set_clipboard (const void *buf, size_t len)
 	}
       clipbuf = (cygcb_t *) GlobalLock (hmem);
 
-      clock_gettime (CLOCK_REALTIME, &clipbuf->timestamp);
+      struct timespec ts;
+      clock_gettime (CLOCK_REALTIME, &ts);
+      clipbuf->tv_sec = ts.tv_sec;
+      clipbuf->tv_nsec = ts.tv_nsec;
       clipbuf->len = len;
       memcpy (clipbuf->data, buf, len);
 
@@ -179,7 +183,10 @@ fhandler_dev_clipboard::fstat (struct stat *buf)
 	  && (hglb = GetClipboardData (format))
 	  && (clipbuf = (cygcb_t *) GlobalLock (hglb)))
 	{
-	  buf->st_atim = buf->st_mtim = clipbuf->timestamp;
+	  struct timespec ts;
+	  ts.tv_sec = clipbuf->tv_sec;
+	  ts.tv_nsec = clipbuf->tv_nsec;
+	  buf->st_atim = buf->st_mtim = ts;
 	  buf->st_size = clipbuf->len;
 	  GlobalUnlock (hglb);
 	}

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit
  2021-10-08  9:52 ` Takashi Yano
@ 2021-10-09 14:19   ` Ken Brown
  2021-10-09 14:29     ` Jon Turney
  0 siblings, 1 reply; 14+ messages in thread
From: Ken Brown @ 2021-10-09 14:19 UTC (permalink / raw)
  To: cygwin-patches

On 10/8/2021 5:52 AM, Takashi Yano wrote:
> How about simply just:
> 
> diff --git a/winsup/cygwin/fhandler_clipboard.cc b/winsup/cygwin/fhandler_clipboard.cc
> index ccdb295f3..d822f4fc4 100644
> --- a/winsup/cygwin/fhandler_clipboard.cc
> +++ b/winsup/cygwin/fhandler_clipboard.cc
> @@ -28,9 +28,10 @@ static const WCHAR *CYGWIN_NATIVE = L"CYGWIN_NATIVE_CLIPBOARD";
>   
>   typedef struct
>   {
> -  timestruc_t	timestamp;
> -  size_t	len;
> -  char		data[1];
> +  uint64_t tv_sec;
> +  uint64_t tv_nsec;
> +  uint64_t len;
> +  char data[1];
>   } cygcb_t;

The only problem with this is that it might leave readers scratching their heads 
unless they look at the commit that introduced this.  What about something like 
the following, in which the code speaks for itself:

diff --git a/winsup/cygwin/fhandler_clipboard.cc 
b/winsup/cygwin/fhandler_clipboard.cc
index ccdb295f3..028c00f1e 100644
--- a/winsup/cygwin/fhandler_clipboard.cc
+++ b/winsup/cygwin/fhandler_clipboard.cc
@@ -26,12 +26,26 @@ details. */

  static const WCHAR *CYGWIN_NATIVE = L"CYGWIN_NATIVE_CLIPBOARD";

+#ifdef __x86_64__
  typedef struct
  {
    timestruc_t  timestamp;
    size_t       len;
    char         data[1];
  } cygcb_t;
+#else
+/* Use same layout. */
+typedef struct
+{
+  struct
+  {
+    int64_t tv_sec;
+    int64_t tv_nsec;
+  }            timestamp;
+  uint64_t     len;
+  char         data[1];
+} cygcb_t;
+#endif

  fhandler_dev_clipboard::fhandler_dev_clipboard ()
    : fhandler_base (), pos (0), membuffer (NULL), msize (0)
@@ -74,7 +88,14 @@ fhandler_dev_clipboard::set_clipboard (const void *buf, 
size_t len)
         }
        clipbuf = (cygcb_t *) GlobalLock (hmem);

+#ifdef __x86_64__
        clock_gettime (CLOCK_REALTIME, &clipbuf->timestamp);
+#else
+      timestruc_t ts;
+      clock_gettime (CLOCK_REALTIME, &ts);
+      clipbuf->timestamp->tv_sec = ts.tv_sec;
+      clipbuf->timestamp->tv_nsec = ts.tv_nsec;
+#endif
        clipbuf->len = len;
        memcpy (clipbuf->data, buf, len);

@@ -179,7 +200,14 @@ fhandler_dev_clipboard::fstat (struct stat *buf)
           && (hglb = GetClipboardData (format))
           && (clipbuf = (cygcb_t *) GlobalLock (hglb)))
         {
+#ifdef __x86_64__
           buf->st_atim = buf->st_mtim = clipbuf->timestamp;
+#else
+         timestruc_t ts;
+         ts.tv_sec = clipbuf->timestamp->tv_sec;
+         ts.tv_nsec = clipbuf->timestamp->tv_nsec;
+         buf->st_atim = buf->st_mtim = ts;
+#endif
           buf->st_size = clipbuf->len;
           GlobalUnlock (hglb);
         }

Ken

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit
  2021-10-09 14:19   ` Ken Brown
@ 2021-10-09 14:29     ` Jon Turney
  2021-10-09 14:43       ` Ken Brown
  0 siblings, 1 reply; 14+ messages in thread
From: Jon Turney @ 2021-10-09 14:29 UTC (permalink / raw)
  To: Cygwin Patches

On 07/10/2021 06:22, Mark Geisert wrote:
 > The cygutils package has two programs, putclip and getclip, that also
 > depend on the layout of the cygcb_t.  At present they have duplicate
 > defs of struct cygcb_t defined here as no Cygwin header provides it.

This struct should maybe be in sys/cygwin.h or similar, if it's expected 
to be used in user-space as well.

On 09/10/2021 15:19, Ken Brown wrote:
> On 10/8/2021 5:52 AM, Takashi Yano wrote:
>> How about simply just:
>>
>> diff --git a/winsup/cygwin/fhandler_clipboard.cc 
>> b/winsup/cygwin/fhandler_clipboard.cc
>> index ccdb295f3..d822f4fc4 100644
>> --- a/winsup/cygwin/fhandler_clipboard.cc
>> +++ b/winsup/cygwin/fhandler_clipboard.cc
>> @@ -28,9 +28,10 @@ static const WCHAR *CYGWIN_NATIVE = 
>> L"CYGWIN_NATIVE_CLIPBOARD";
>>   typedef struct
>>   {
>> -  timestruc_t    timestamp;
>> -  size_t    len;
>> -  char        data[1];
>> +  uint64_t tv_sec;
>> +  uint64_t tv_nsec;
>> +  uint64_t len;
>> +  char data[1];
>>   } cygcb_t;
> 
> The only problem with this is that it might leave readers scratching 
> their heads unless they look at the commit that introduced this.  What 

I think the solution to that is a "comment" like "we don't use 'struct 
timespec' here as it's different size on different arches and that 
causes problem XYZ".

(I think that's preferable to conditional code which we assert (but 
don't ensure) is the same on all arches)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit
  2021-10-09 14:29     ` Jon Turney
@ 2021-10-09 14:43       ` Ken Brown
  2021-10-11  6:13         ` Mark Geisert
  0 siblings, 1 reply; 14+ messages in thread
From: Ken Brown @ 2021-10-09 14:43 UTC (permalink / raw)
  To: cygwin-patches

On 10/9/2021 10:29 AM, Jon Turney wrote:
> On 07/10/2021 06:22, Mark Geisert wrote:
>  > The cygutils package has two programs, putclip and getclip, that also
>  > depend on the layout of the cygcb_t.  At present they have duplicate
>  > defs of struct cygcb_t defined here as no Cygwin header provides it.
> 
> This struct should maybe be in sys/cygwin.h or similar, if it's expected to be 
> used in user-space as well.

Good idea.

> On 09/10/2021 15:19, Ken Brown wrote:
>> On 10/8/2021 5:52 AM, Takashi Yano wrote:
>>> How about simply just:
>>>
>>> diff --git a/winsup/cygwin/fhandler_clipboard.cc 
>>> b/winsup/cygwin/fhandler_clipboard.cc
>>> index ccdb295f3..d822f4fc4 100644
>>> --- a/winsup/cygwin/fhandler_clipboard.cc
>>> +++ b/winsup/cygwin/fhandler_clipboard.cc
>>> @@ -28,9 +28,10 @@ static const WCHAR *CYGWIN_NATIVE = 
>>> L"CYGWIN_NATIVE_CLIPBOARD";
>>>   typedef struct
>>>   {
>>> -  timestruc_t    timestamp;
>>> -  size_t    len;
>>> -  char        data[1];
>>> +  uint64_t tv_sec;
>>> +  uint64_t tv_nsec;
>>> +  uint64_t len;
>>> +  char data[1];
>>>   } cygcb_t;
>>
>> The only problem with this is that it might leave readers scratching their 
>> heads unless they look at the commit that introduced this.  What 
> 
> I think the solution to that is a "comment" like "we don't use 'struct timespec' 
> here as it's different size on different arches and that causes problem XYZ".

And the same for size_t.

Ken

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit
  2021-10-09 14:43       ` Ken Brown
@ 2021-10-11  6:13         ` Mark Geisert
  2021-10-11 12:11           ` Ken Brown
  0 siblings, 1 reply; 14+ messages in thread
From: Mark Geisert @ 2021-10-11  6:13 UTC (permalink / raw)
  To: cygwin-patches

Ken Brown wrote:
> On 10/9/2021 10:29 AM, Jon Turney wrote:
[...]
>>> On 10/8/2021 5:52 AM, Takashi Yano wrote:
[...]

Thanks all for the comments; much appreciated.  They'll be factored into v2 in one 
form or another.  I pronounced my original patch "bad" not because of any problem 
with code operation or struct cygcb_t definition.  I used anonymous union and 
anonymous struct internally to mostly realize what Takashi suggested for layout, 
just naming the items cb_* rather than tv_* or other.  The code works as intended, 
32- and 64-bit.

It's just that after submitting the patch I realized that, if we really are going 
to support both Cygwin archs (x86_64 and i686), there is still the issue of 
different cygcb_t layouts between Cygwin versions being ignored.

Specifically, the fhandler_clipboard::fstat routine can't tell which Cygwin 
environment has set the clipboard contents.  My original patch takes care of 
32-bit and 64-bit, providing both are running Cygwin >= 3.3.0 (presumably).  What 
if it was a different version (pre 3.3.0) that set the contents?

So I'm working on a heuristic to identify which cygcb_t layout is present in the 
clipboard data.  This will hopefully distinguish between the 3 historical cygcb_t 
layouts as well as x86_64 differing from i686 for each one.
Stay tuned,

..mark

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit
  2021-10-11  6:13         ` Mark Geisert
@ 2021-10-11 12:11           ` Ken Brown
  2021-10-22 15:11             ` Corinna Vinschen
  0 siblings, 1 reply; 14+ messages in thread
From: Ken Brown @ 2021-10-11 12:11 UTC (permalink / raw)
  To: cygwin-patches

On 10/11/2021 2:13 AM, Mark Geisert wrote:
> It's just that after submitting the patch I realized that, if we really are 
> going to support both Cygwin archs (x86_64 and i686), there is still the issue 
> of different cygcb_t layouts between Cygwin versions being ignored.
> 
> Specifically, the fhandler_clipboard::fstat routine can't tell which Cygwin 
> environment has set the clipboard contents.  My original patch takes care of 
> 32-bit and 64-bit, providing both are running Cygwin >= 3.3.0 (presumably).  
> What if it was a different version (pre 3.3.0) that set the contents?

I wonder if this is worth the trouble.  Right now we have a problem in which 
data written to /dev/clipboard in one arch can't be read in the other arch.  The 
fix will appear in Cygwin 3.3.0.  Do we really have to try to make the fix apply 
retroactively in case the user updates one arch but not the other?

Ken

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit
  2021-10-11 12:11           ` Ken Brown
@ 2021-10-22 15:11             ` Corinna Vinschen
  2021-10-23  5:35               ` Mark Geisert
  2021-10-23 23:58               ` Takashi Yano
  0 siblings, 2 replies; 14+ messages in thread
From: Corinna Vinschen @ 2021-10-22 15:11 UTC (permalink / raw)
  To: cygwin-patches

On Oct 11 08:11, Ken Brown wrote:
> On 10/11/2021 2:13 AM, Mark Geisert wrote:
> > It's just that after submitting the patch I realized that, if we really
> > are going to support both Cygwin archs (x86_64 and i686), there is still
> > the issue of different cygcb_t layouts between Cygwin versions being
> > ignored.
> > 
> > Specifically, the fhandler_clipboard::fstat routine can't tell which
> > Cygwin environment has set the clipboard contents.  My original patch
> > takes care of 32-bit and 64-bit, providing both are running Cygwin >=
> > 3.3.0 (presumably).  What if it was a different version (pre 3.3.0) that
> > set the contents?
> 
> I wonder if this is worth the trouble.  Right now we have a problem in which
> data written to /dev/clipboard in one arch can't be read in the other arch.
> The fix will appear in Cygwin 3.3.0.  Do we really have to try to make the
> fix apply retroactively in case the user updates one arch but not the other?

Just to close this up prior to the 3.3.0 release...

Given we never actually strived for 32<->64 bit interoperability, it's
hard to argue why this should be different for the clipboard stuff.

Running 32 and 64 bit Cygwin versions in parallel doesn't actually make
much sense for most people anyway, unless they explicitely develop for
32 and 64 bit systems under Cygwin.  From a productivity point of view
there's no good reason to run more than one arch.

So I agree with Ken here.  It's probably not worth the trouble.


Corinna

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit
  2021-10-22 15:11             ` Corinna Vinschen
@ 2021-10-23  5:35               ` Mark Geisert
  2021-10-23 20:32                 ` Ken Brown
  2021-10-23 23:58               ` Takashi Yano
  1 sibling, 1 reply; 14+ messages in thread
From: Mark Geisert @ 2021-10-23  5:35 UTC (permalink / raw)
  To: cygwin-patches

Hi all,

Corinna Vinschen wrote:
> On Oct 11 08:11, Ken Brown wrote:
>> On 10/11/2021 2:13 AM, Mark Geisert wrote:
>>> It's just that after submitting the patch I realized that, if we really
>>> are going to support both Cygwin archs (x86_64 and i686), there is still
>>> the issue of different cygcb_t layouts between Cygwin versions being
>>> ignored.
>>>
>>> Specifically, the fhandler_clipboard::fstat routine can't tell which
>>> Cygwin environment has set the clipboard contents.  My original patch
>>> takes care of 32-bit and 64-bit, providing both are running Cygwin >=
>>> 3.3.0 (presumably).  What if it was a different version (pre 3.3.0) that
>>> set the contents?
>>
>> I wonder if this is worth the trouble.  Right now we have a problem in which
>> data written to /dev/clipboard in one arch can't be read in the other arch.
>> The fix will appear in Cygwin 3.3.0.  Do we really have to try to make the
>> fix apply retroactively in case the user updates one arch but not the other?
> 
> Just to close this up prior to the 3.3.0 release...
> 
> Given we never actually strived for 32<->64 bit interoperability, it's
> hard to argue why this should be different for the clipboard stuff.
> 
> Running 32 and 64 bit Cygwin versions in parallel doesn't actually make
> much sense for most people anyway, unless they explicitely develop for
> 32 and 64 bit systems under Cygwin.  From a productivity point of view
> there's no good reason to run more than one arch.
> 
> So I agree with Ken here.  It's probably not worth the trouble.

Sorry, I've been sidetracked for a bit.  I can agree with Ken too.  The only 
circumstance I could think of where multiple internal format support might be 
useful (to non-developers) was some user hanging on to an older Cygwin because it 
was needed to support something else (s/w or h/w) old and non-upgradeable. 
Doesn't seem very likely at this point.

I'll try to get the v2 patch out over this weekend.  Same end-result for same 
environments as the v1 patch, but incorporating all the comments I received.

To that end, does Jon's suggestion of /usr/include/sys/cygwin.h seem like the best 
location to define struct cygcb_t for use by both Cygwin and cygutils package?
Thanks much,

..mark

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit
  2021-10-23  5:35               ` Mark Geisert
@ 2021-10-23 20:32                 ` Ken Brown
  2021-10-23 21:53                   ` Mark Geisert
  0 siblings, 1 reply; 14+ messages in thread
From: Ken Brown @ 2021-10-23 20:32 UTC (permalink / raw)
  To: cygwin-patches

On 10/23/2021 1:35 AM, Mark Geisert wrote:
> Corinna Vinschen wrote:
>> Just to close this up prior to the 3.3.0 release...
>>
>> Given we never actually strived for 32<->64 bit interoperability, it's
>> hard to argue why this should be different for the clipboard stuff.
>>
>> Running 32 and 64 bit Cygwin versions in parallel doesn't actually make
>> much sense for most people anyway, unless they explicitely develop for
>> 32 and 64 bit systems under Cygwin.  From a productivity point of view
>> there's no good reason to run more than one arch.
>>
>> So I agree with Ken here.  It's probably not worth the trouble.
> 
> Sorry, I've been sidetracked for a bit.  I can agree with Ken too.  The only 
> circumstance I could think of where multiple internal format support might be 
> useful (to non-developers) was some user hanging on to an older Cygwin because 
> it was needed to support something else (s/w or h/w) old and non-upgradeable. 
> Doesn't seem very likely at this point.
> 
> I'll try to get the v2 patch out over this weekend.  Same end-result for same 
> environments as the v1 patch, but incorporating all the comments I received.

I think Corinna was saying that the whole idea of making the 32-bit and 64-bit 
clipboards interoperable is not worth the trouble.

> To that end, does Jon's suggestion of /usr/include/sys/cygwin.h seem like the 
> best location to define struct cygcb_t for use by both Cygwin and cygutils package?
> Thanks much,
> 
> ..mark

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit
  2021-10-23 20:32                 ` Ken Brown
@ 2021-10-23 21:53                   ` Mark Geisert
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Geisert @ 2021-10-23 21:53 UTC (permalink / raw)
  To: cygwin-patches

Ken Brown wrote:
> On 10/23/2021 1:35 AM, Mark Geisert wrote:
>> Corinna Vinschen wrote:
>>> Just to close this up prior to the 3.3.0 release...
>>>
>>> Given we never actually strived for 32<->64 bit interoperability, it's
>>> hard to argue why this should be different for the clipboard stuff.
>>>
>>> Running 32 and 64 bit Cygwin versions in parallel doesn't actually make
>>> much sense for most people anyway, unless they explicitely develop for
>>> 32 and 64 bit systems under Cygwin.  From a productivity point of view
>>> there's no good reason to run more than one arch.
>>>
>>> So I agree with Ken here.  It's probably not worth the trouble.
>>
>> Sorry, I've been sidetracked for a bit.  I can agree with Ken too.  The only 
>> circumstance I could think of where multiple internal format support might be 
>> useful (to non-developers) was some user hanging on to an older Cygwin because 
>> it was needed to support something else (s/w or h/w) old and non-upgradeable. 
>> Doesn't seem very likely at this point.
>>
>> I'll try to get the v2 patch out over this weekend.  Same end-result for same 
>> environments as the v1 patch, but incorporating all the comments I received.
> 
> I think Corinna was saying that the whole idea of making the 32-bit and 64-bit 
> clipboards interoperable is not worth the trouble.

Oh, I didn't read it that way.  But that works for me too.  Color it dropped :-).
Thanks,

..mark

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit
  2021-10-22 15:11             ` Corinna Vinschen
  2021-10-23  5:35               ` Mark Geisert
@ 2021-10-23 23:58               ` Takashi Yano
  2021-10-25  8:28                 ` Corinna Vinschen
  1 sibling, 1 reply; 14+ messages in thread
From: Takashi Yano @ 2021-10-23 23:58 UTC (permalink / raw)
  To: cygwin-patches

Hi Corinna,

On Fri, 22 Oct 2021 17:11:13 +0200
Corinna Vinschen wrote:
> Just to close this up prior to the 3.3.0 release...
> 
> Given we never actually strived for 32<->64 bit interoperability, it's
> hard to argue why this should be different for the clipboard stuff.
> 
> Running 32 and 64 bit Cygwin versions in parallel doesn't actually make
> much sense for most people anyway, unless they explicitely develop for
> 32 and 64 bit systems under Cygwin.  From a productivity point of view
> there's no good reason to run more than one arch.
> 
> So I agree with Ken here.  It's probably not worth the trouble.

Current code below in fhandler_clipboard.cc causes access violation
if clipboard is accessed between 32 and 64 bit cygwin.

      cygcb_t *clipbuf = (cygcb_t *) cb_data;

      if (pos < (off_t) clipbuf->len)
        {
          ret = ((len > (clipbuf->len - pos)) ? (clipbuf->len - pos) : len);
          memcpy (ptr, clipbuf->data + pos , ret);
          pos += ret;
        }

Don't you think this should be fixed?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit
  2021-10-23 23:58               ` Takashi Yano
@ 2021-10-25  8:28                 ` Corinna Vinschen
  0 siblings, 0 replies; 14+ messages in thread
From: Corinna Vinschen @ 2021-10-25  8:28 UTC (permalink / raw)
  To: cygwin-patches

On Oct 24 08:58, Takashi Yano wrote:
> Hi Corinna,
> 
> On Fri, 22 Oct 2021 17:11:13 +0200
> Corinna Vinschen wrote:
> > Just to close this up prior to the 3.3.0 release...
> > 
> > Given we never actually strived for 32<->64 bit interoperability, it's
> > hard to argue why this should be different for the clipboard stuff.
> > 
> > Running 32 and 64 bit Cygwin versions in parallel doesn't actually make
> > much sense for most people anyway, unless they explicitely develop for
> > 32 and 64 bit systems under Cygwin.  From a productivity point of view
> > there's no good reason to run more than one arch.
> > 
> > So I agree with Ken here.  It's probably not worth the trouble.
> 
> Current code below in fhandler_clipboard.cc causes access violation
> if clipboard is accessed between 32 and 64 bit cygwin.
> 
>       cygcb_t *clipbuf = (cygcb_t *) cb_data;
> 
>       if (pos < (off_t) clipbuf->len)
>         {
>           ret = ((len > (clipbuf->len - pos)) ? (clipbuf->len - pos) : len);
>           memcpy (ptr, clipbuf->data + pos , ret);
>           pos += ret;
>         }
> 
> Don't you think this should be fixed?

Well, if you put it this way...

I didn't get that it's crashing, so yeah, in that case a fix would
be nice, of course.


Corinna

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-10-25  8:28 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-07  5:22 [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit Mark Geisert
2021-10-07  5:56 ` Mark Geisert
2021-10-08  9:52 ` Takashi Yano
2021-10-09 14:19   ` Ken Brown
2021-10-09 14:29     ` Jon Turney
2021-10-09 14:43       ` Ken Brown
2021-10-11  6:13         ` Mark Geisert
2021-10-11 12:11           ` Ken Brown
2021-10-22 15:11             ` Corinna Vinschen
2021-10-23  5:35               ` Mark Geisert
2021-10-23 20:32                 ` Ken Brown
2021-10-23 21:53                   ` Mark Geisert
2021-10-23 23:58               ` Takashi Yano
2021-10-25  8:28                 ` Corinna Vinschen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).