GNU bug report logs - #58341
[PATCH] Add x11vnc recipe

Previous Next

Package: guix-patches;

Reported by: Mehmet Tekman <mtekman89 <at> gmail.com>

Date: Thu, 6 Oct 2022 17:32:02 UTC

Severity: normal

Tags: moreinfo, patch

Done: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 58341 in the body.
You can then email your comments to 58341 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#58341; Package guix-patches. (Thu, 06 Oct 2022 17:32:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Mehmet Tekman <mtekman89 <at> gmail.com>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Thu, 06 Oct 2022 17:32:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Mehmet Tekman <mtekman89 <at> gmail.com>
To: guix-patches <at> gnu.org
Subject: [PATCH] Add x11vnc recipe
Date: Thu, 6 Oct 2022 18:05:31 +0200
---
 gnu/packages/patches/x11vnc-gcc10-fix.patch   | 42 ++++++++++
 ...c-scan-limit-access-to-shared-memory.patch | 22 ++++++
 .../patches/x11vnc-xfc-null-ptr.patch         | 29 +++++++
 gnu/packages/x11vnc.scm                       | 79 +++++++++++++++++++
 4 files changed, 172 insertions(+)
 create mode 100644 gnu/packages/patches/x11vnc-gcc10-fix.patch
 create mode 100644
gnu/packages/patches/x11vnc-scan-limit-access-to-shared-memory.patch
 create mode 100644 gnu/packages/patches/x11vnc-xfc-null-ptr.patch
 create mode 100644 gnu/packages/x11vnc.scm

diff --git a/gnu/packages/patches/x11vnc-gcc10-fix.patch
b/gnu/packages/patches/x11vnc-gcc10-fix.patch
new file mode 100644
index 0000000000..a32d5785d9
--- /dev/null
+++ b/gnu/packages/patches/x11vnc-gcc10-fix.patch
@@ -0,0 +1,42 @@
+From a48b0b1cd887d7f3ae67f525d7d334bd2feffe60 Mon Sep 17 00:00:00 2001
+From: Alexander Tsoy <alexander <at> tsoy.me>
+Date: Tue, 28 Jan 2020 22:21:01 +0300
+Subject: [PATCH] Fix build with -fno-common
+
+GCC 10 defaults to -fno-common
+---
+ src/util.c | 3 +++
+ src/util.h | 6 +++---
+ 2 files changed, 6 insertions(+), 3 deletions(-)
+
+diff --git a/src/util.c b/src/util.c
+index a82a1a42..6a52ebf4 100644
+--- a/src/util.c
++++ b/src/util.c
+@@ -47,6 +47,9 @@ int hxl = 0;
+ #ifdef LIBVNCSERVER_HAVE_LIBPTHREAD
+ MUTEX(x11Mutex);
+ MUTEX(scrollMutex);
++MUTEX(clientMutex);
++MUTEX(inputMutex);
++MUTEX(pointerMutex);
+ #endif
+
+ int nfix(int i, int n);
+diff --git a/src/util.h b/src/util.h
+index 35c1afd2..99b5dd1d 100644
+--- a/src/util.h
++++ b/src/util.h
+@@ -102,9 +102,9 @@ extern struct timeval _mysleep;
+ #ifdef LIBVNCSERVER_HAVE_LIBPTHREAD
+ extern MUTEX(x11Mutex);
+ extern MUTEX(scrollMutex);
+-MUTEX(clientMutex);
+-MUTEX(inputMutex);
+-MUTEX(pointerMutex);
++extern MUTEX(clientMutex);
++extern MUTEX(inputMutex);
++extern MUTEX(pointerMutex);
+ #endif
+
+ #define X_INIT INIT_MUTEX(x11Mutex)
diff --git a/gnu/packages/patches/x11vnc-scan-limit-access-to-shared-memory.patch
b/gnu/packages/patches/x11vnc-scan-limit-access-to-shared-memory.patch
new file mode 100644
index 0000000000..5424094434
--- /dev/null
+++ b/gnu/packages/patches/x11vnc-scan-limit-access-to-shared-memory.patch
@@ -0,0 +1,22 @@
+From 69eeb9f7baa14ca03b16c9de821f9876def7a36a Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Gu=C3=A9nal=20DAVALAN?= <guenal.davalan <at> uca.fr>
+Date: Wed, 18 Nov 2020 08:40:45 +0100
+Subject: [PATCH] scan: limit access to shared memory segments to current user
+
+---
+ src/scan.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/src/scan.c b/src/scan.c
+index 43e00d20..12994d52 100644
+--- a/src/scan.c
++++ b/src/scan.c
+@@ -320,7 +320,7 @@ static int shm_create(XShmSegmentInfo *shm,
XImage **ximg_ptr, int w, int h,
+
+ #if HAVE_XSHM
+     shm->shmid = shmget(IPC_PRIVATE,
+-        xim->bytes_per_line * xim->height, IPC_CREAT | 0777);
++        xim->bytes_per_line * xim->height, IPC_CREAT | 0600);
+
+     if (shm->shmid == -1) {
+         rfbErr("shmget(%s) failed.\n", name);
diff --git a/gnu/packages/patches/x11vnc-xfc-null-ptr.patch
b/gnu/packages/patches/x11vnc-xfc-null-ptr.patch
new file mode 100644
index 0000000000..65f339d716
--- /dev/null
+++ b/gnu/packages/patches/x11vnc-xfc-null-ptr.patch
@@ -0,0 +1,29 @@
+From 95a10ab64c2dbbec2c8dad91a5ffb73a0d68474b Mon Sep 17 00:00:00 2001
+From: Jonathan Liu <net147 <at> gmail.com>
+Date: Mon, 16 Mar 2020 20:04:06 +1100
+Subject: [PATCH] src/cursor: fix xfc NULL pointer dereference
+
+xfc->width and xfc->height for the XFixes cursor image returned from
+XFixesGetCursorImage(dpy) are accessed without first checking that xfc
+is not NULL. This can result in the server sometimes crashing when
+moving a Google Chrome window.
+
+Fixes: 37c946191a0f ("Broken cursor bugfix for 64 bit systems (#49)")
+Signed-off-by: Jonathan Liu <net147 <at> gmail.com>
+---
+ src/cursor.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/src/cursor.c b/src/cursor.c
+index 39e73a69..74a08c69 100644
+--- a/src/cursor.c
++++ b/src/cursor.c
+@@ -1311,7 +1311,7 @@ static int get_exact_cursor(int init) {
+
+         /* retrieve the cursor info + pixels from server: */
+         xfc = XFixesGetCursorImage(dpy);
+-        {
++        if (xfc) {
+             /* 2017-07-09, Stephan Fuhrmann: This fixes an
implementation flaw for 64 bit systems.
+              * The XFixesCursorImage structure says xfc->pixels is
(unsigned long*) in the structure, but
+              * the protocol spec says it's 32 bit per pixel
diff --git a/gnu/packages/x11vnc.scm b/gnu/packages/x11vnc.scm
new file mode 100644
index 0000000000..55c51305b3
--- /dev/null
+++ b/gnu/packages/x11vnc.scm
@@ -0,0 +1,79 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2022 Mehmet Tekman <mtekman89 <at> gmail.com>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+
+(define-module (gnu packages x11vnc)
+  #:use-module (guix packages)
+  #:use-module (gnu packages)
+  #:use-module (gnu packages perl)
+  #:use-module (gnu packages tcl)
+  #:use-module (gnu packages pkg-config)
+  #:use-module (gnu packages xorg)
+  #:use-module (gnu packages tls)
+  #:use-module (guix git-download)
+  #:use-module (guix build-system gnu)
+  #:use-module ((guix licenses) #:prefix license:)
+  #:use-module (gnu packages autotools)
+  #:use-module (gnu packages avahi)
+  #:use-module (gnu packages vnc)
+  #:use-module (gnu packages compression)
+  #:use-module (gnu packages image))
+
+(define-public x11vnc
+  (let ((commit "4ca006fed80410bd9b061a1519bd5d9366bb0bc8")
+        (version "0.9.16")
+        (revision "1"))
+    (package
+      (name "x11vnc")
+      (version (git-version version revision commit))
+      (source (origin
+                (method git-fetch)
+                (uri (git-reference
+                      (url "https://github.com/LibVNC/x11vnc")
+                      (commit commit)))
+                (file-name (git-file-name name version))
+                (sha256
+                 (base32
+                  "1g652mmi79pfq4p5p7spaswa164rpzjhc5rn2phy5pm71lm0vib1"))
+                (patches
+                 (search-patches "x11vnc-xfc-null-ptr.patch"
+                                 "x11vnc-gcc10-fix.patch"
+
"x11vnc-scan-limit-access-to-shared-memory.patch"))))
+      (build-system gnu-build-system)
+      (arguments
+       '(#:phases
+         (modify-phases
+             %standard-phases
+           (add-before 'bootstrap 'delete-premature-configure
+             (lambda _ (substitute* "./autogen.sh"
+                    ((".*/configure") "")))))))
+      (native-inputs (list
+                      ;; [optional requirements]
+                      ;; perl tk
+                      autoconf automake autobuild pkg-config avahi libvnc
+                      libx11 libxcomposite libxdamage libxext libxfixes libxi
+                      libxinerama libxrandr libxtst
+                      openssl xdpyinfo zlib libjpeg-turbo xf86-video-dummy))
+      (synopsis "VNC server for real X displays")
+      (home-page "https://github.com/LibVNC/x11vnc")
+      (description
+       "x11vnc allows one to view remotely and interact with real X
+displays (i.e. a display corresponding to a physical monitor, keyboard, and
+mouse) with any VNC viewer.  In this way it plays the role for Unix/X11 that
+WinVNC plays for Windows.")
+      (license license:gpl2))))
-- 
2.38.0




Information forwarded to guix-patches <at> gnu.org:
bug#58341; Package guix-patches. (Sat, 08 Oct 2022 04:34:01 GMT) Full text and rfc822 format available.

Message #8 received at 58341 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Mehmet Tekman <mtekman89 <at> gmail.com>
Cc: 58341 <at> debbugs.gnu.org
Subject: Re: bug#58341: [PATCH] Add x11vnc recipe
Date: Sat, 08 Oct 2022 00:33:46 -0400
Hello, and thank you for your contribution!

Mehmet Tekman <mtekman89 <at> gmail.com> writes:

> ---
>  gnu/packages/patches/x11vnc-gcc10-fix.patch   | 42 ++++++++++
>  ...c-scan-limit-access-to-shared-memory.patch | 22 ++++++
>  .../patches/x11vnc-xfc-null-ptr.patch         | 29 +++++++
>  gnu/packages/x11vnc.scm                       | 79 +++++++++++++++++++
>  4 files changed, 172 insertions(+)
>  create mode 100644 gnu/packages/patches/x11vnc-gcc10-fix.patch
>  create mode 100644
> gnu/packages/patches/x11vnc-scan-limit-access-to-shared-memory.patch
>  create mode 100644 gnu/packages/patches/x11vnc-xfc-null-ptr.patch
>  create mode 100644 gnu/packages/x11vnc.scm

Would it be possible to send the patch complete with the commit message,
as can be produced with 'git format-patch'?  You can read 'info "(guix)
Submitting Patches".  The git commit message should use the GNU
ChangeLog format, as mentioned there.  You can look at the git history
to learn.

The patches should be registered in the gnu/local.mk file, in the
dist_patch_DATA section.  Please mind the sorting in the file.

> diff --git a/gnu/packages/patches/x11vnc-gcc10-fix.patch
> b/gnu/packages/patches/x11vnc-gcc10-fix.patch
> new file mode 100644
> index 0000000000..a32d5785d9
> --- /dev/null
> +++ b/gnu/packages/patches/x11vnc-gcc10-fix.patch
> @@ -0,0 +1,42 @@
> +From a48b0b1cd887d7f3ae67f525d7d334bd2feffe60 Mon Sep 17 00:00:00 2001
> +From: Alexander Tsoy <alexander <at> tsoy.me>
> +Date: Tue, 28 Jan 2020 22:21:01 +0300
> +Subject: [PATCH] Fix build with -fno-common
> +
> +GCC 10 defaults to -fno-common
> +---
> + src/util.c | 3 +++
> + src/util.h | 6 +++---
> + 2 files changed, 6 insertions(+), 3 deletions(-)
> +
> +diff --git a/src/util.c b/src/util.c
> +index a82a1a42..6a52ebf4 100644
> +--- a/src/util.c
> ++++ b/src/util.c
> +@@ -47,6 +47,9 @@ int hxl = 0;
> + #ifdef LIBVNCSERVER_HAVE_LIBPTHREAD
> + MUTEX(x11Mutex);
> + MUTEX(scrollMutex);
> ++MUTEX(clientMutex);
> ++MUTEX(inputMutex);
> ++MUTEX(pointerMutex);
> + #endif
> +
> + int nfix(int i, int n);
> +diff --git a/src/util.h b/src/util.h
> +index 35c1afd2..99b5dd1d 100644
> +--- a/src/util.h
> ++++ b/src/util.h
> +@@ -102,9 +102,9 @@ extern struct timeval _mysleep;
> + #ifdef LIBVNCSERVER_HAVE_LIBPTHREAD
> + extern MUTEX(x11Mutex);
> + extern MUTEX(scrollMutex);
> +-MUTEX(clientMutex);
> +-MUTEX(inputMutex);
> +-MUTEX(pointerMutex);
> ++extern MUTEX(clientMutex);
> ++extern MUTEX(inputMutex);
> ++extern MUTEX(pointerMutex);
> + #endif
> +
> + #define X_INIT INIT_MUTEX(x11Mutex)
> diff --git a/gnu/packages/patches/x11vnc-scan-limit-access-to-shared-memory.patch
> b/gnu/packages/patches/x11vnc-scan-limit-access-to-shared-memory.patch
> new file mode 100644
> index 0000000000..5424094434
> --- /dev/null
> +++ b/gnu/packages/patches/x11vnc-scan-limit-access-to-shared-memory.patch
> @@ -0,0 +1,22 @@
> +From 69eeb9f7baa14ca03b16c9de821f9876def7a36a Mon Sep 17 00:00:00 2001
> +From: =?UTF-8?q?Gu=C3=A9nal=20DAVALAN?= <guenal.davalan <at> uca.fr>
> +Date: Wed, 18 Nov 2020 08:40:45 +0100
> +Subject: [PATCH] scan: limit access to shared memory segments to current user
> +
> +---
> + src/scan.c | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/src/scan.c b/src/scan.c
> +index 43e00d20..12994d52 100644
> +--- a/src/scan.c
> ++++ b/src/scan.c
> +@@ -320,7 +320,7 @@ static int shm_create(XShmSegmentInfo *shm,
> XImage **ximg_ptr, int w, int h,
> +
> + #if HAVE_XSHM
> +     shm->shmid = shmget(IPC_PRIVATE,
> +-        xim->bytes_per_line * xim->height, IPC_CREAT | 0777);
> ++        xim->bytes_per_line * xim->height, IPC_CREAT | 0600);
> +
> +     if (shm->shmid == -1) {
> +         rfbErr("shmget(%s) failed.\n", name);
> diff --git a/gnu/packages/patches/x11vnc-xfc-null-ptr.patch
> b/gnu/packages/patches/x11vnc-xfc-null-ptr.patch
> new file mode 100644
> index 0000000000..65f339d716
> --- /dev/null
> +++ b/gnu/packages/patches/x11vnc-xfc-null-ptr.patch
> @@ -0,0 +1,29 @@
> +From 95a10ab64c2dbbec2c8dad91a5ffb73a0d68474b Mon Sep 17 00:00:00 2001
> +From: Jonathan Liu <net147 <at> gmail.com>
> +Date: Mon, 16 Mar 2020 20:04:06 +1100
> +Subject: [PATCH] src/cursor: fix xfc NULL pointer dereference
> +
> +xfc->width and xfc->height for the XFixes cursor image returned from
> +XFixesGetCursorImage(dpy) are accessed without first checking that xfc
> +is not NULL. This can result in the server sometimes crashing when
> +moving a Google Chrome window.
> +
> +Fixes: 37c946191a0f ("Broken cursor bugfix for 64 bit systems (#49)")
> +Signed-off-by: Jonathan Liu <net147 <at> gmail.com>
> +---
> + src/cursor.c | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/src/cursor.c b/src/cursor.c
> +index 39e73a69..74a08c69 100644
> +--- a/src/cursor.c
> ++++ b/src/cursor.c
> +@@ -1311,7 +1311,7 @@ static int get_exact_cursor(int init) {
> +
> +         /* retrieve the cursor info + pixels from server: */
> +         xfc = XFixesGetCursorImage(dpy);
> +-        {
> ++        if (xfc) {
> +             /* 2017-07-09, Stephan Fuhrmann: This fixes an
> implementation flaw for 64 bit systems.
> +              * The XFixesCursorImage structure says xfc->pixels is
> (unsigned long*) in the structure, but
> +              * the protocol spec says it's 32 bit per pixel
> diff --git a/gnu/packages/x11vnc.scm b/gnu/packages/x11vnc.scm
> new file mode 100644
> index 0000000000..55c51305b3

Since we are using a recent commit, shouldn't these (upstream?) patches
be unnecessary?  Perhaps we can use an even newer commit that includes
them?

> --- /dev/null
> +++ b/gnu/packages/x11vnc.scm
> @@ -0,0 +1,79 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2022 Mehmet Tekman <mtekman89 <at> gmail.com>
> +;;;
> +;;; This file is part of GNU Guix.
> +;;;
> +;;; GNU Guix is free software; you can redistribute it and/or modify it
> +;;; under the terms of the GNU General Public License as published by
> +;;; the Free Software Foundation; either version 3 of the License, or (at
> +;;; your option) any later version.
> +;;;
> +;;; GNU Guix is distributed in the hope that it will be useful, but
> +;;; WITHOUT ANY WARRANTY; without even the implied warranty of
> +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +;;; GNU General Public License for more details.
> +;;;
> +;;; You should have received a copy of the GNU General Public License
> +;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
> +
> +
> +(define-module (gnu packages x11vnc)

There already is a (gnu packages vnc) module; that'd be a better home.
We can group all VNC-related packages there.

> +  #:use-module (guix packages)
> +  #:use-module (gnu packages)
> +  #:use-module (gnu packages perl)
> +  #:use-module (gnu packages tcl)
> +  #:use-module (gnu packages pkg-config)
> +  #:use-module (gnu packages xorg)
> +  #:use-module (gnu packages tls)
> +  #:use-module (guix git-download)
> +  #:use-module (guix build-system gnu)
> +  #:use-module ((guix licenses) #:prefix license:)
> +  #:use-module (gnu packages autotools)
> +  #:use-module (gnu packages avahi)
> +  #:use-module (gnu packages vnc)
> +  #:use-module (gnu packages compression)
> +  #:use-module (gnu packages image))

Imports should be lexicographically sorted.

> +(define-public x11vnc

When using an unreleased version, it's good to mention why (issues it
fixes, a very old release, etc.).
> +  (let ((commit "4ca006fed80410bd9b061a1519bd5d9366bb0bc8")
> +        (version "0.9.16")
> +        (revision "1"))
> +    (package
> +      (name "x11vnc")
> +      (version (git-version version revision commit))
> +      (source (origin
> +                (method git-fetch)
> +                (uri (git-reference
> +                      (url "https://github.com/LibVNC/x11vnc")
> +                      (commit commit)))
> +                (file-name (git-file-name name version))
> +                (sha256
> +                 (base32
> +                  "1g652mmi79pfq4p5p7spaswa164rpzjhc5rn2phy5pm71lm0vib1"))
> +                (patches
> +                 (search-patches "x11vnc-xfc-null-ptr.patch"
> +                                 "x11vnc-gcc10-fix.patch"
> +
> "x11vnc-scan-limit-access-to-shared-memory.patch"))))

Odd blank line here.  You can try './pre-inst-env guix style x11vnc' to
see how it would do it, else I'd do it like to avoid breaking the 80
chars max width:

(search-patches
 "patch1"
 "patch2"
 ...)

> +      (build-system gnu-build-system)
> +      (arguments
> +       '(#:phases
> +         (modify-phases
> +             %standard-phases
> +           (add-before 'bootstrap 'delete-premature-configure
> +             (lambda _ (substitute* "./autogen.sh"
> +                    ((".*/configure") "")))))))
> +      (native-inputs (list
> +                      ;; [optional requirements]
> +                      ;; perl tk
> +                      autoconf automake autobuild pkg-config avahi libvnc
> +                      libx11 libxcomposite libxdamage libxext libxfixes libxi
> +                      libxinerama libxrandr libxtst
> +                      openssl xdpyinfo zlib libjpeg-turbo xf86-video-dummy))

The libraries that are used at run time should appear as inputs, not
native-inputs.  Native-inputs are for build tools or tests.

> +      (synopsis "VNC server for real X displays")
> +      (home-page "https://github.com/LibVNC/x11vnc")
> +      (description
> +       "x11vnc allows one to view remotely and interact with real X
> +displays (i.e. a display corresponding to a physical monitor, keyboard, and
> +mouse) with any VNC viewer.  In this way it plays the role for Unix/X11 that
> +WinVNC plays for Windows.")

I'd remove the useless comparison with windows, perhaps stress that this
provides a VNC *server* in the description as well.

> +      (license license:gpl2))))

-- 
Thanks,
Maxim




Added tag(s) moreinfo. Request was from Maxim Cournoyer <maxim.cournoyer <at> gmail.com> to control <at> debbugs.gnu.org. (Sat, 08 Oct 2022 04:35:02 GMT) Full text and rfc822 format available.

Information forwarded to guix-patches <at> gnu.org:
bug#58341; Package guix-patches. (Mon, 10 Oct 2022 16:07:01 GMT) Full text and rfc822 format available.

Message #13 received at 58341 <at> debbugs.gnu.org (full text, mbox):

From: Mehmet Tekman <mtekman89 <at> gmail.com>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 58341 <at> debbugs.gnu.org
Subject: Re: bug#58341: [PATCH] Add x11vnc recipe
Date: Mon, 10 Oct 2022 18:05:49 +0200
[Message part 1 (text/plain, inline)]
Hello, thank you for the review!

I've put the recipe into vnc.scm and built to a newer commit that
makes the patches redundant (thanks for the tip, I was basing my
recipe off one from another distro).

I've fixed the commit message and linted and styled the vnc.scm file.

I created a new branch and put my changes there, so please disregard
the above changes.

I attach here the patch file, as I don't quite trust myself to paste
the changes into this email.

Looking forward to the next review,

Best,
Mehmet

On Sat, 8 Oct 2022 at 06:33, Maxim Cournoyer <maxim.cournoyer <at> gmail.com> wrote:
>
> Hello, and thank you for your contribution!
>
> Mehmet Tekman <mtekman89 <at> gmail.com> writes:
>
> > ---
> >  gnu/packages/patches/x11vnc-gcc10-fix.patch   | 42 ++++++++++
> >  ...c-scan-limit-access-to-shared-memory.patch | 22 ++++++
> >  .../patches/x11vnc-xfc-null-ptr.patch         | 29 +++++++
> >  gnu/packages/x11vnc.scm                       | 79 +++++++++++++++++++
> >  4 files changed, 172 insertions(+)
> >  create mode 100644 gnu/packages/patches/x11vnc-gcc10-fix.patch
> >  create mode 100644
> > gnu/packages/patches/x11vnc-scan-limit-access-to-shared-memory.patch
> >  create mode 100644 gnu/packages/patches/x11vnc-xfc-null-ptr.patch
> >  create mode 100644 gnu/packages/x11vnc.scm
>
> Would it be possible to send the patch complete with the commit message,
> as can be produced with 'git format-patch'?  You can read 'info "(guix)
> Submitting Patches".  The git commit message should use the GNU
> ChangeLog format, as mentioned there.  You can look at the git history
> to learn.
>
> The patches should be registered in the gnu/local.mk file, in the
> dist_patch_DATA section.  Please mind the sorting in the file.
>
> > diff --git a/gnu/packages/patches/x11vnc-gcc10-fix.patch
> > b/gnu/packages/patches/x11vnc-gcc10-fix.patch
> > new file mode 100644
> > index 0000000000..a32d5785d9
> > --- /dev/null
> > +++ b/gnu/packages/patches/x11vnc-gcc10-fix.patch
> > @@ -0,0 +1,42 @@
> > +From a48b0b1cd887d7f3ae67f525d7d334bd2feffe60 Mon Sep 17 00:00:00 2001
> > +From: Alexander Tsoy <alexander <at> tsoy.me>
> > +Date: Tue, 28 Jan 2020 22:21:01 +0300
> > +Subject: [PATCH] Fix build with -fno-common
> > +
> > +GCC 10 defaults to -fno-common
> > +---
> > + src/util.c | 3 +++
> > + src/util.h | 6 +++---
> > + 2 files changed, 6 insertions(+), 3 deletions(-)
> > +
> > +diff --git a/src/util.c b/src/util.c
> > +index a82a1a42..6a52ebf4 100644
> > +--- a/src/util.c
> > ++++ b/src/util.c
> > +@@ -47,6 +47,9 @@ int hxl = 0;
> > + #ifdef LIBVNCSERVER_HAVE_LIBPTHREAD
> > + MUTEX(x11Mutex);
> > + MUTEX(scrollMutex);
> > ++MUTEX(clientMutex);
> > ++MUTEX(inputMutex);
> > ++MUTEX(pointerMutex);
> > + #endif
> > +
> > + int nfix(int i, int n);
> > +diff --git a/src/util.h b/src/util.h
> > +index 35c1afd2..99b5dd1d 100644
> > +--- a/src/util.h
> > ++++ b/src/util.h
> > +@@ -102,9 +102,9 @@ extern struct timeval _mysleep;
> > + #ifdef LIBVNCSERVER_HAVE_LIBPTHREAD
> > + extern MUTEX(x11Mutex);
> > + extern MUTEX(scrollMutex);
> > +-MUTEX(clientMutex);
> > +-MUTEX(inputMutex);
> > +-MUTEX(pointerMutex);
> > ++extern MUTEX(clientMutex);
> > ++extern MUTEX(inputMutex);
> > ++extern MUTEX(pointerMutex);
> > + #endif
> > +
> > + #define X_INIT INIT_MUTEX(x11Mutex)
> > diff --git a/gnu/packages/patches/x11vnc-scan-limit-access-to-shared-memory.patch
> > b/gnu/packages/patches/x11vnc-scan-limit-access-to-shared-memory.patch
> > new file mode 100644
> > index 0000000000..5424094434
> > --- /dev/null
> > +++ b/gnu/packages/patches/x11vnc-scan-limit-access-to-shared-memory.patch
> > @@ -0,0 +1,22 @@
> > +From 69eeb9f7baa14ca03b16c9de821f9876def7a36a Mon Sep 17 00:00:00 2001
> > +From: =?UTF-8?q?Gu=C3=A9nal=20DAVALAN?= <guenal.davalan <at> uca.fr>
> > +Date: Wed, 18 Nov 2020 08:40:45 +0100
> > +Subject: [PATCH] scan: limit access to shared memory segments to current user
> > +
> > +---
> > + src/scan.c | 2 +-
> > + 1 file changed, 1 insertion(+), 1 deletion(-)
> > +
> > +diff --git a/src/scan.c b/src/scan.c
> > +index 43e00d20..12994d52 100644
> > +--- a/src/scan.c
> > ++++ b/src/scan.c
> > +@@ -320,7 +320,7 @@ static int shm_create(XShmSegmentInfo *shm,
> > XImage **ximg_ptr, int w, int h,
> > +
> > + #if HAVE_XSHM
> > +     shm->shmid = shmget(IPC_PRIVATE,
> > +-        xim->bytes_per_line * xim->height, IPC_CREAT | 0777);
> > ++        xim->bytes_per_line * xim->height, IPC_CREAT | 0600);
> > +
> > +     if (shm->shmid == -1) {
> > +         rfbErr("shmget(%s) failed.\n", name);
> > diff --git a/gnu/packages/patches/x11vnc-xfc-null-ptr.patch
> > b/gnu/packages/patches/x11vnc-xfc-null-ptr.patch
> > new file mode 100644
> > index 0000000000..65f339d716
> > --- /dev/null
> > +++ b/gnu/packages/patches/x11vnc-xfc-null-ptr.patch
> > @@ -0,0 +1,29 @@
> > +From 95a10ab64c2dbbec2c8dad91a5ffb73a0d68474b Mon Sep 17 00:00:00 2001
> > +From: Jonathan Liu <net147 <at> gmail.com>
> > +Date: Mon, 16 Mar 2020 20:04:06 +1100
> > +Subject: [PATCH] src/cursor: fix xfc NULL pointer dereference
> > +
> > +xfc->width and xfc->height for the XFixes cursor image returned from
> > +XFixesGetCursorImage(dpy) are accessed without first checking that xfc
> > +is not NULL. This can result in the server sometimes crashing when
> > +moving a Google Chrome window.
> > +
> > +Fixes: 37c946191a0f ("Broken cursor bugfix for 64 bit systems (#49)")
> > +Signed-off-by: Jonathan Liu <net147 <at> gmail.com>
> > +---
> > + src/cursor.c | 2 +-
> > + 1 file changed, 1 insertion(+), 1 deletion(-)
> > +
> > +diff --git a/src/cursor.c b/src/cursor.c
> > +index 39e73a69..74a08c69 100644
> > +--- a/src/cursor.c
> > ++++ b/src/cursor.c
> > +@@ -1311,7 +1311,7 @@ static int get_exact_cursor(int init) {
> > +
> > +         /* retrieve the cursor info + pixels from server: */
> > +         xfc = XFixesGetCursorImage(dpy);
> > +-        {
> > ++        if (xfc) {
> > +             /* 2017-07-09, Stephan Fuhrmann: This fixes an
> > implementation flaw for 64 bit systems.
> > +              * The XFixesCursorImage structure says xfc->pixels is
> > (unsigned long*) in the structure, but
> > +              * the protocol spec says it's 32 bit per pixel
> > diff --git a/gnu/packages/x11vnc.scm b/gnu/packages/x11vnc.scm
> > new file mode 100644
> > index 0000000000..55c51305b3
>
> Since we are using a recent commit, shouldn't these (upstream?) patches
> be unnecessary?  Perhaps we can use an even newer commit that includes
> them?
>
> > --- /dev/null
> > +++ b/gnu/packages/x11vnc.scm
> > @@ -0,0 +1,79 @@
> > +;;; GNU Guix --- Functional package management for GNU
> > +;;; Copyright © 2022 Mehmet Tekman <mtekman89 <at> gmail.com>
> > +;;;
> > +;;; This file is part of GNU Guix.
> > +;;;
> > +;;; GNU Guix is free software; you can redistribute it and/or modify it
> > +;;; under the terms of the GNU General Public License as published by
> > +;;; the Free Software Foundation; either version 3 of the License, or (at
> > +;;; your option) any later version.
> > +;;;
> > +;;; GNU Guix is distributed in the hope that it will be useful, but
> > +;;; WITHOUT ANY WARRANTY; without even the implied warranty of
> > +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +;;; GNU General Public License for more details.
> > +;;;
> > +;;; You should have received a copy of the GNU General Public License
> > +;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
> > +
> > +
> > +(define-module (gnu packages x11vnc)
>
> There already is a (gnu packages vnc) module; that'd be a better home.
> We can group all VNC-related packages there.
>
> > +  #:use-module (guix packages)
> > +  #:use-module (gnu packages)
> > +  #:use-module (gnu packages perl)
> > +  #:use-module (gnu packages tcl)
> > +  #:use-module (gnu packages pkg-config)
> > +  #:use-module (gnu packages xorg)
> > +  #:use-module (gnu packages tls)
> > +  #:use-module (guix git-download)
> > +  #:use-module (guix build-system gnu)
> > +  #:use-module ((guix licenses) #:prefix license:)
> > +  #:use-module (gnu packages autotools)
> > +  #:use-module (gnu packages avahi)
> > +  #:use-module (gnu packages vnc)
> > +  #:use-module (gnu packages compression)
> > +  #:use-module (gnu packages image))
>
> Imports should be lexicographically sorted.
>
> > +(define-public x11vnc
>
> When using an unreleased version, it's good to mention why (issues it
> fixes, a very old release, etc.).
> > +  (let ((commit "4ca006fed80410bd9b061a1519bd5d9366bb0bc8")
> > +        (version "0.9.16")
> > +        (revision "1"))
> > +    (package
> > +      (name "x11vnc")
> > +      (version (git-version version revision commit))
> > +      (source (origin
> > +                (method git-fetch)
> > +                (uri (git-reference
> > +                      (url "https://github.com/LibVNC/x11vnc")
> > +                      (commit commit)))
> > +                (file-name (git-file-name name version))
> > +                (sha256
> > +                 (base32
> > +                  "1g652mmi79pfq4p5p7spaswa164rpzjhc5rn2phy5pm71lm0vib1"))
> > +                (patches
> > +                 (search-patches "x11vnc-xfc-null-ptr.patch"
> > +                                 "x11vnc-gcc10-fix.patch"
> > +
> > "x11vnc-scan-limit-access-to-shared-memory.patch"))))
>
> Odd blank line here.  You can try './pre-inst-env guix style x11vnc' to
> see how it would do it, else I'd do it like to avoid breaking the 80
> chars max width:
>
> (search-patches
>  "patch1"
>  "patch2"
>  ...)
>
> > +      (build-system gnu-build-system)
> > +      (arguments
> > +       '(#:phases
> > +         (modify-phases
> > +             %standard-phases
> > +           (add-before 'bootstrap 'delete-premature-configure
> > +             (lambda _ (substitute* "./autogen.sh"
> > +                    ((".*/configure") "")))))))
> > +      (native-inputs (list
> > +                      ;; [optional requirements]
> > +                      ;; perl tk
> > +                      autoconf automake autobuild pkg-config avahi libvnc
> > +                      libx11 libxcomposite libxdamage libxext libxfixes libxi
> > +                      libxinerama libxrandr libxtst
> > +                      openssl xdpyinfo zlib libjpeg-turbo xf86-video-dummy))
>
> The libraries that are used at run time should appear as inputs, not
> native-inputs.  Native-inputs are for build tools or tests.
>
> > +      (synopsis "VNC server for real X displays")
> > +      (home-page "https://github.com/LibVNC/x11vnc")
> > +      (description
> > +       "x11vnc allows one to view remotely and interact with real X
> > +displays (i.e. a display corresponding to a physical monitor, keyboard, and
> > +mouse) with any VNC viewer.  In this way it plays the role for Unix/X11 that
> > +WinVNC plays for Windows.")
>
> I'd remove the useless comparison with windows, perhaps stress that this
> provides a VNC *server* in the description as well.
>
> > +      (license license:gpl2))))
>
> --
> Thanks,
> Maxim
[0001-gnu-Add-x11vnc.patch (text/x-patch, attachment)]

Information forwarded to guix-patches <at> gnu.org:
bug#58341; Package guix-patches. (Tue, 11 Oct 2022 10:54:02 GMT) Full text and rfc822 format available.

Message #16 received at 58341 <at> debbugs.gnu.org (full text, mbox):

From: Mehmet Tekman <mtekman89 <at> gmail.com>
To: 58341 <at> debbugs.gnu.org
Subject: Re: bug#58341: [PATCH] Add x11vnc recipe
Date: Tue, 11 Oct 2022 12:52:26 +0200
(Apologies for the quoted reply, I've now learned how to disable that)




Reply sent to Maxim Cournoyer <maxim.cournoyer <at> gmail.com>:
You have taken responsibility. (Mon, 31 Oct 2022 15:37:02 GMT) Full text and rfc822 format available.

Notification sent to Mehmet Tekman <mtekman89 <at> gmail.com>:
bug acknowledged by developer. (Mon, 31 Oct 2022 15:37:02 GMT) Full text and rfc822 format available.

Message #21 received at 58341-done <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Mehmet Tekman <mtekman89 <at> gmail.com>
Cc: 58341-done <at> debbugs.gnu.org
Subject: Re: bug#58341: [PATCH] Add x11vnc recipe
Date: Mon, 31 Oct 2022 11:36:37 -0400
Hi Mehmet,

Mehmet Tekman <mtekman89 <at> gmail.com> writes:

> Hello, thank you for the review!

Thanks for the prompt reply, and sorry for the delays on my part!

> I've put the recipe into vnc.scm and built to a newer commit that
> makes the patches redundant (thanks for the tip, I was basing my
> recipe off one from another distro).
>
> I've fixed the commit message and linted and styled the vnc.scm file.
>
> I created a new branch and put my changes there, so please disregard
> the above changes.
>
> I attach here the patch file, as I don't quite trust myself to paste
> the changes into this email.

That's fine.  It looks very good!  I've made the following
cosmetic/nitpicking changes:

--8<---------------cut here---------------start------------->8---
modified   gnu/packages/vnc.scm
@@ -568,11 +568,10 @@ (define-public x11vnc
   ;; The release version of 0.9.16 requires patches to work, so we pin to the
   ;; latest working commit
   (let ((commit "3e4dc8ef2985a6e670e1d9649fe55395c2b31039")
-        (version "0.9.16")
-        (revision "1"))
+        (revision "0"))
     (package
       (name "x11vnc")
-      (version (git-version version revision commit))
+      (version (git-version "0.9.16" revision commit))
       (source (origin
                 (method git-fetch)
                 (uri (git-reference
@@ -584,14 +583,15 @@ (define-public x11vnc
                   "0a120gv9h3whiznlddl0j3nz3400jjgl97znaincm5i2m5pnjifs"))))
       (build-system gnu-build-system)
       (arguments
-       '(#:phases (modify-phases %standard-phases
-                    (add-before 'bootstrap 'delete-premature-configure
-                      (lambda _
-                        (substitute* "./autogen.sh"
-                          ((".*/configure")
-                           "")))))))
+       (list #:phases #~(modify-phases %standard-phases
+                          (add-before 'bootstrap 'delete-premature-configure
+                            (lambda _
+                              (substitute* "./autogen.sh"
+                                ((".*/configure")
+                                 "")))))))
       (native-inputs (list autoconf automake autobuild pkg-config))
       (inputs (list avahi
+                    libjpeg-turbo
                     libvnc
                     libx11
                     libxcomposite
@@ -604,9 +604,8 @@ (define-public x11vnc
                     libxtst
                     openssl
                     xdpyinfo
-                    zlib
-                    libjpeg-turbo
-                    xf86-video-dummy))
+                    xf86-video-dummy
+                    zlib))
       (synopsis "VNC server for real X displays")
       (home-page "https://github.com/LibVNC/x11vnc")
       (description
@@ -615,7 +614,6 @@ (define-public x11vnc
 mouse) with any VNC viewer.")
       (license license:gpl2))))
 
-
 (define-public libvnc
   (package
     (name "libvnc")
--8<---------------cut here---------------end--------------->8---

1. Do not bind a "version" variable in let; it's bound by the version
field of the package record at definition (see: define-record-type* from
(guix records)).

2. Used a plain list with a gexp for the phases, which is more
forward-looking.

3. Sorted the inputs.

4. Removed an extra newline at the end (we use only one newline between
definitions).

Before pushing, I've also double checked the license and found that it
should be gpl2+, as the COPYING file contains the "any later version"
text.  x11vnc.h also has in the license notice:

    x11vnc is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
    the Free Software Foundation; either version 2 of the License, or (at
    your option) any later version.

I made these changes and pushed to master!

Thanks again,

Maxim




Information forwarded to guix-patches <at> gnu.org:
bug#58341; Package guix-patches. (Mon, 31 Oct 2022 17:31:01 GMT) Full text and rfc822 format available.

Message #24 received at 58341-done <at> debbugs.gnu.org (full text, mbox):

From: Mehmet Tekman <mtekman89 <at> gmail.com>
To: 58341-done <at> debbugs.gnu.org
Subject: Re: bug#58341: [PATCH] Add x11vnc recipe
Date: Mon, 31 Oct 2022 18:30:15 +0100
[Message part 1 (text/plain, inline)]
Thanks for the edits!
[Message part 2 (text/html, inline)]

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 29 Nov 2022 12:24:06 GMT) Full text and rfc822 format available.

This bug report was last modified 1 year and 149 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.