Good evening fellow hackers,

two patches I already made a while ago, but just got around to publish
here.

The first is to refactor the dontkillme function with very strict error
checking and using file pointers rather than file descriptors, as
recommended by the Linux kernel team for accessing the procfs.
The second is converting the manpage to mandoc using the proven and
tested format used across suckless tools (sbase, ubase, sent,
farbfeld, ...) and beyond. Also the example was improved.

As food for thought, writing the manual page I noticed that we might
want a flag "-p" for slock to drop privileges before running the
command. s2ram might need root privileges, but it might be that a user
wants to run a simple command not needing root privileges.
Keeping this in mind, the usage string was altered from the short form
to the "extended" form so if we add a new flag in the future we won't
have to do it there.

Cheers

FRIGN

-- 
FRIGN <[email protected]>
>From e18e79e0e0575ef4cdeaaf91f5c99b363d736dd2 Mon Sep 17 00:00:00 2001
From: FRIGN <[email protected]>
Date: Tue, 23 Aug 2016 01:45:46 +0200
Subject: [PATCH 1/2] Refactor dontkillme()

- Use file pointers instead of raw I/O, inspired by Kernel code.
- Use OOM_SCORE_ADJ_MIN from linux/oom.h instead of working with
  magic values.
- Stricter error checking and descriptive error messages.

The reasoning for using the constant rather than magic values lies
in the fact that this ensures people get the message.
With "-1000", a code reviewer would question if that is really the
lowest possible number or just an arbitrary value.
The kernel ABI probably won't change, but even in the case, we wouldn't
have to modify the code. The OOM killer only is guaranteed to not
kill you if you have OOM_SCORE_ADJ_MIN.
---
 slock.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/slock.c b/slock.c
index 210d5c8..63f8d4a 100644
--- a/slock.c
+++ b/slock.c
@@ -65,19 +65,27 @@ die(const char *errstr, ...)
 
 #ifdef __linux__
 #include <fcntl.h>
+#include <linux/oom.h>
 
 static void
 dontkillme(void)
 {
-	int fd;
+	FILE *f;
+	const char oomfile[] = "/proc/self/oom_score_adj";
 
-	fd = open("/proc/self/oom_score_adj", O_WRONLY);
-	if (fd < 0 && errno == ENOENT) {
-		return;
+	if (!(f = fopen(oomfile, "w"))) {
+		if (errno == ENOENT)
+			return;
+		die("slock: fopen %s: %s\n", oomfile, strerror(errno));
 	}
-	if (fd < 0 || write(fd, "-1000\n", (sizeof("-1000\n") - 1)) !=
-	    (sizeof("-1000\n") - 1) || close(fd) != 0) {
-		die("can't tame the oom-killer. is suid or sgid set?\n");
+	fprintf(f, "%d", OOM_SCORE_ADJ_MIN);
+	if (fclose(f)) {
+		if (errno == EACCES)
+			die("slock: unable to disable OOM killer. "
+			    "suid or sgid set?\n");
+		else
+			die("slock: fclose %s: %s\n", oomfile,
+			    strerror(errno));
 	}
 }
 #endif
-- 
2.7.3

>From 4424501c874891928a7eb760c0cb16d0347ca10d Mon Sep 17 00:00:00 2001
From: FRIGN <[email protected]>
Date: Tue, 23 Aug 2016 10:55:34 +0200
Subject: [PATCH 2/2] Convert manpage to mandoc and fix usage

In all honor, the previous usage was formally more correct, but for the
sake of consistency across all the tools having the v-flag, I separated
it from the command-string.

Also, make use of the mandoc macros for the manpage. This makes it
easier to maintain, extend and change in the future.
---
 slock.1 | 54 ++++++++++++++++++++++++++----------------------------
 slock.c |  2 +-
 2 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/slock.1 b/slock.1
index 0ef3e15..2b2b7c8 100644
--- a/slock.1
+++ b/slock.1
@@ -1,29 +1,27 @@
-.TH SLOCK 1 slock\-VERSION
-.SH NAME
-slock \- simple X screen locker
-.SH SYNOPSIS
-.B slock
-.RB [ \-v
-|
-.IR cmd ]
-.SH DESCRIPTION
-.B slock
-is an X screen locker. If provided,
-.IR cmd
+.Dd 2016-08-23
+.Dt SLOCK 1
+.Sh NAME
+.Nm slock
+.Nd simple X screen locker
+.Sh SYNOPSIS
+.Nm
+.Op Fl v
+.Op Ar cmd Op Ar arg ...
+.Sh DESCRIPTION
+.Nm
+is a simple X screen locker. If provided,
+.Ar cmd Op Ar arg ...
 is executed after the screen has been locked.
-.SH OPTIONS
-.TP
-.B \-v
-prints version information to stdout, then exits.
-.SH EXAMPLES
-$ slock /usr/sbin/s2ram
-.SH CUSTOMIZATION
-.B slock
-can be customized by creating a custom config.h and (re)compiling the source
-code. This keeps it fast, secure and simple.
-.SH AUTHORS
-See the LICENSE file for the authors.
-.SH LICENSE
-See the LICENSE file for the terms of redistribution.
-.SH BUGS
-Please report them.
+.Sh OPTIONS
+.Bl -tag -width Ds
+.It Fl v
+Print version information to stdout and exit.
+.El
+.Sh EXAMPLES
+$
+.Nm
+/usr/sbin/s2ram
+.Sh CUSTOMIZATION
+.Nm
+can be customized by creating a custom config.h from config.def.h and
+(re)compiling the source code. This keeps it fast, secure and simple.
diff --git a/slock.c b/slock.c
index 63f8d4a..0ae748f 100644
--- a/slock.c
+++ b/slock.c
@@ -290,7 +290,7 @@ lockscreen(Display *dpy, int screen)
 static void
 usage(void)
 {
-	die("usage: slock [-v | cmd [arg ...]]\n");
+	die("usage: slock [-v] [cmd [arg ...]]\n");
 }
 
 int
-- 
2.7.3

Reply via email to