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 wrote: > > Hello, and thank you for your contribution! > > Mehmet Tekman 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 > > +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?= > > +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 > > +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 > > +--- > > + 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 > > +;;; > > +;;; 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 . > > + > > + > > +(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