Re: [hackers] [sbase][patches] sort makefile and add getconf guard file
On Mon, 3 Oct 2016 15:59:10 -0700 Evan Gates <evan.ga...@gmail.com> wrote: Hey Evan, > After discussion on IRC, a patch that changes getconf.sh and creates a > single header. I have pushed your patch and one little tweak for the heredocs to the 2f30 staging repository. They'll hit mainline soon. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [sbase][patch]find: copy path before using basename
On Mon, 3 Oct 2016 15:04:16 -0700 Evan Gates <evan.ga...@gmail.com> wrote: > "The basename() function may modify the string pointed to by path..." > Thanks POSIX. > + char path[strlen(arg->path)+1]; > + strcpy(path, arg->path); > + return !fnmatch((char *)arg->extra.p, path, 0); Please don't use VLA's. Use estrdup() in this case. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [sbase][patches] sort makefile and add getconf guard file
On Thu, 29 Sep 2016 08:54:30 -0700 Evan Gates <evan.ga...@gmail.com> wrote: Hey Evan, > sbase-sort_makefile.diff sorts all the lists in the Makefile. Sorry > that was bugging me. this looks good. :) > sbase-getconf_guard.diff adds a guard file to the getconf rules so > that getconf.sh doesn't get run multiple times in parallel. This could be solved a bit more elegantly. You can see that getconf.sh generates the files confstr_l.h, limits_l.h, sysconf_l.h and pathconf_l.h. Maybe you can work something out. :) Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [PATCH] [slock] Remove faulty example and add a section on security considerations
On Wed, 28 Sep 2016 22:04:05 +0200 Klemens Nanni <k...@posteo.org> wrote: Hey Klemens, > It's implicitly blocked by capturing the keys being pressed and > executing optional steps such as shutting down upon input. what if somebody remaps this functionality? This is possible with Xkb sadly. > Fair point, I shall fix this. You cannot fix this at this level. As Markus already pointed out, you have to carefully handle the options not to "destroy" the layout at the end. Also, what if some automated script changes the options regularly? You cannot cache the options reliably. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [PATCH] [slock] Remove faulty example and add a section on security considerations
On Wed, 28 Sep 2016 21:41:36 +0200
Klemens Nanni <k...@posteo.org> wrote:
Hey Klemens,
> I removed media upload and SMS support since those features can easily
> be added using a small wrapper script.
I don't see the gain anyway with that but to each his own. If somebody
tried to access my computer, it gives the red color, which is
sufficient. All the cruft introduced with these changes just makes
slock more insecure.
> Setting `DontVTSwitch' in xorg.conf(5) disables this feature
> completely whereas chjj's fork (which mine is based on) blocks it in
> slock only, which is imho a much saner approach since there are many
> legitimate reasons to use multiple virtual terminals.
Can you point me to the piece of code that disables VT-switching in
your fork? I couldn't find it.
> Same story for `DontZap': I like quickly killing X with Ctrl+Alt+BS
> while this should obviously be forbidden on a locked screen.
What you do is call
system("doas setxkbmap -option &");
which disables Ctrl+Alt+Backspace for the entire session. So you can
only kill your X server until you have locked your screen once. It
won't work afterwards, which sucks and is unpredictable.
Cheers
FRIGN
--
FRIGN <d...@frign.de>
Re: [hackers] [PATCH] [slock] Remove faulty example and add a section on security considerations
On Wed, 28 Sep 2016 21:17:23 +0200 Markus Teich <markus.te...@stusta.mhn.de> wrote: Hey Markus, > I don't think it is that obvious. Have a look at the discussion > starting from the slock-1.3 announcement on February 12th this year > again. Since the example does not work any more, changing it to > `slock sudo s2ram` and adding a note about the needed line in the > sudo config so s2ram can be run without a password would be better. the problem doesn't end there. Also, s2ram is Linux specific and in 99% of the cases you run unprivileged after-lock commands. To be honest, I expect any half-decent user how to set up doas or sudo. > The other block looks good and is more simple and flexible than > changing the xkb config ourselves from slock.c. Yes, the problem really is that if we did the xkb changes within slock.c, we somehow would have to cache the option string. If for any reason it is changed while slock is running (automation, whatever), and we uncache it, this can lead to strange behaviour. Cheers FRIGN -- FRIGN <d...@frign.de>
[hackers] [PATCH] [slock] Remove faulty example and add a section on security considerations
Hello fellow hackers, the question has been floating around for quite some time on the internet, but I think it is a good place to answer it in the manual of our screen locker. Is slock really secure and if not, how can I harden it so that nobody can access my machine? There are two ways one can possibly circumvent a locked X screen (not including security holes in the Kernel) 1) switch to a different VT that is logged in. Then there, proceed to kill slock and switch back the now unlocked VT. 2) kill the X server with Ctrl+Alt+Backspace (if enabled). If no login manager is used, this yields an open shell. All work within the X session is usually lost, but the attacker still has access to the user data. Sysrq can be used to kill all running processes, but this also logs out the user and thus is no problem. I did not add it here because if somebody wants to "pwn" the user he can just unplug the computer or take out the battery to destroy all the work. You can disable VT switching and Ctrl+Alt+Backspace (this also overrides the local Xkb settings and is thus foolproof) for sure by setting two options in xorg.conf. See the patch for details on the instructions. Cheers FRIGN -- FRIGN <d...@frign.de> >From 2e363c4dfc98153f8067df27673dda9047ab9227 Mon Sep 17 00:00:00 2001 From: FRIGN <d...@frign.de> Date: Wed, 28 Sep 2016 20:20:51 +0200 Subject: [PATCH] Remove faulty example and add a section on security considerations The given example does not work and the usage is so obvious that an example probably is not necessary here anyway. The section on security considerations sheds some light on the problems that we can't solve within slock but which the user has to solve in his X configuration. --- slock.1 | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/slock.1 b/slock.1 index 2b2b7c8..6c1e8bd 100644 --- a/slock.1 +++ b/slock.1 @@ -17,10 +17,18 @@ is executed after the screen has been locked. .It Fl v Print version information to stdout and exit. .El -.Sh EXAMPLES -$ -.Nm -/usr/sbin/s2ram +.Sh SECURITY CONSIDERATIONS +To make sure a locked screen can not be bypassed by switching VTs +or killing the X server with Ctrl+Alt+Backspace, it is recommended +to disable both in +.Xr xorg.conf 5 +for maximum security: +.Bd -literal -offset left +Section "ServerFlags" + Option "DontVTSwitch" "True" + Option "DontZap" "True" +EndSection +.Ed .Sh CUSTOMIZATION .Nm can be customized by creating a custom config.h from config.def.h and -- 2.7.3
[hackers] [sinit] [PATCH] Use switch for fork()
Hello fellow hackers,
I spotted this small detail this morning and applied this change. It is
always a good thing in my opinion to be able to carve off an
unnecessary local variable.
To be precise, I think this can be considered the first patch of this
year's slcon3 hacking sessions. ;)
With best regards
FRIGN
--
FRIGN <d...@frign.de>
>From 6365fab78c11e8269447cf0cfcbb1aeac618488b Mon Sep 17 00:00:00 2001
From: FRIGN <d...@frign.de>
Date: Fri, 23 Sep 2016 09:37:59 +0200
Subject: [PATCH] Use switch for fork()
This saves us one local variable and 2 lines of code, while improving
readability by using the switch-style we are used to from other suckless
projects.
We are allowed to check against -1, as POSIX clearly mandates for the
RETURN VALUE:
"Upon successful completion, fork() shall return 0 to the child process
and shall return the process ID of the child process to the parent
process. Both processes shall continue to execute from the fork()
function. Otherwise, -1 shall be returned to the parent process, no
child process shall be created, and errno shall be set to indicate the
error."
[http://pubs.opengroup.org/onlinepubs/9699919799/functions/fork.html]
This way, checking against < 0 was overdoing it and it's sufficient to
compare against -1, justifying the switch statement here.
---
sinit.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/sinit.c b/sinit.c
index e338f35..93f9925 100644
--- a/sinit.c
+++ b/sinit.c
@@ -74,16 +74,14 @@ sigreboot(void)
static void
spawn(char *const argv[])
{
- pid_t pid;
-
- pid = fork();
- if (pid < 0) {
- perror("fork");
- } else if (pid == 0) {
+ switch (fork()) {
+ case 0:
sigprocmask(SIG_UNBLOCK, , NULL);
setsid();
execvp(argv[0], argv);
perror("execvp");
_exit(1);
+ case -1:
+ perror("fork");
}
}
--
2.7.3
Re: [hackers] [slock] [PATCHSET] Some improvements and more security
On Mon, 12 Sep 2016 13:13:39 +0200
Quentin Rameau <quinq@fifth.space> wrote:
Hey Quentin,
> That was just one example of the rest of the complexity it brings in
> here.
it just stops hiding the fact that readpw() does many things at once.
It would make sense to split this function apart a bit, maybe separating
the hash-checking from the actual X stuff. Having global state skews
your vision and the biggest sin is having functions which do too many
things at once.
> > because it would be inconsistent with the xrandr-struct.
> It would be inconsistent with… An inconsistency you're bringing in? ^^
what other choice do I have? I could of course typedef the xrandr-struct
typedef struct _xrandr {
...
} Xrandr;
but this would be confusing as hell, given people could almost start
believing this struct was part of X.
> As I said, personal taste.
We had the discussion already. Markus decides in the end.
> Not *that* reasoning, as I already said, I already told you I was for
> getting rid of it.
> I mean the reasoning of why it was introduced in the first place, to
> be sure something is not missed here.
It was added in commit 0f1157d[0] and carried through history since
then. I bet back then it was laziness not wanting to run getpwuid to
get the pw_name entry. There is no reasoning and we should definitely
move away from the $USER environment variable, especially because it
makes slock's behaviour inconsistent across operating systems (in some
cases it is sensitive to $USER, in others it is not).
> Get rid of it.
I'll send in a small patch when I have the time.
Cheers
FRIGN
[0]:http://git.suckless.org/slock/commit/slock.c?id=0f1157d7e6cabe0394ffc35a0a58a30507af8ebd
--
FRIGN <d...@frign.de>
Re: [hackers] [slock] [PATCHSET] Some improvements and more security
On Mon, 12 Sep 2016 12:20:51 +0200 Quentin Rameau <quinq@fifth.space> wrote: Hey Quentin, > First patch: > - I agree with the structuring of the xrandr part, good! > - But not with the localization of every data, they're used across the > whole program and you're just over-complicating functions here imho. > The program is simple and clear enough not to have to get triple > pointers passed as function parameters. the triple pointer is just used once in the cleanup function, as after freeing the locks we set the locks double pointer to NULL. > - un-typedef, that's not an issue here, why changing it besides > satisfying your personal taste? because it would be inconsistent with the xrandr-struct. There is also no reason to typedef struct anyway and just hides the underlying machinery for no good reason. > Second patch: > - fine by me, renaming variables patch ;p You wouldn't believe how much difference it makes to improve variable naming. > Third patch: > - the rval renaming belongs to the second patch It's debatable if you think of the second patch as a renaming patch, but in the end I don't see it as a big issue. > - I agree with the removal of the $USER, we discussed it before this > patch on IRC. But maybe we should have the reasoning behind that > from the original author who put it here. The reasoning is that we don't want a bloody seduid program depending so strongly on environment variables. For some things, it's sufficient to just kick them out because they are so obviously wrong. What has been bugging me for quite a while is this DPMS comment that was added there for no reason. Every sane mind would agree that fiddling with DPMS makes no sense whatsoever. When I slock, my screen turns off after 10 minutes. So, if I don't like that, I disable DPMS. If I do, I just fiddle around with my mouse a bit and get the slock promt. The only issue I see there is if the screen is not locked and one assumes it is locked and does not wait for the screen to turn back on and enter his password, possibly unleashing it to the public (IRC chat, whatever). But this is a minor thing imho. DPMS stays out and there's no need imho for such a long comment to reflect that. > Anyway, thanks for the work! I was happy to do it! :) Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [slock] [PATCHSET] Some improvements and more security
On Sun, 11 Sep 2016 23:24:20 +0200 FRIGN <d...@frign.de> wrote: > See attached. Most important is the patch which removes the > abomination of user $USER which actually poses quite a risk and only > is done on part of the systems. So you can test this, do the following $ unset USER $ slock Segmentation fault $ This is due to getenv() returning NULL which in turn is not well-taken by the getpwnam_user-function. We don't want it to segfault! -- FRIGN <d...@frign.de>
[hackers] [slock] [PATCHSET] Some improvements and more security
Good evening fellow hackers,
I sat down this evening to write down some patches that have been
floating around in my head for a while.
See attached. Most important is the patch which removes the abomination
of user $USER which actually poses quite a risk and only is done on
part of the systems. The largest one is the one removing global state
of the program to make code audits simpler.
Cheers
FRIGN
--
FRIGN <d...@frign.de>
>From 86c4a4edc72958461c9142d218ac0a285751e708 Mon Sep 17 00:00:00 2001
From: FRIGN <d...@frign.de>
Date: Sun, 11 Sep 2016 23:08:19 +0200
Subject: [PATCH 1/3] Localize and rework data structures
For a project like slock there is no need to carry around global state.
By moving the three structures to main() it is now clear which functions
modify which state, greatly improving the readability of the code,
especially given slock is a suid program.
---
slock.c | 62 ++
1 file changed, 34 insertions(+), 28 deletions(-)
diff --git a/slock.c b/slock.c
index 7127ebe..83f6810 100644
--- a/slock.c
+++ b/slock.c
@@ -33,18 +33,18 @@ enum {
#include "config.h"
-typedef struct {
+struct lock {
int screen;
Window root, win;
Pixmap pmap;
unsigned long colors[NUMCOLS];
-} Lock;
+};
-static Lock **locks;
-static int nscreens;
-static Bool rr;
-static int rrevbase;
-static int rrerrbase;
+struct xrandr {
+ int active;
+ int evbase;
+ int errbase;
+};
static void
die(const char *errstr, ...)
@@ -123,7 +123,8 @@ getpw(void)
}
static void
-readpw(Display *dpy, const char *pws)
+readpw(Display *dpy, struct xrandr *rr, struct lock **locks, int nscreens,
+ const char *pws)
{
char buf[32], passwd[256], *encrypted;
int num, screen, running, failure;
@@ -194,7 +195,7 @@ readpw(Display *dpy, const char *pws)
}
oldc = color;
}
- } else if (rr && ev.type == rrevbase + RRScreenChangeNotify) {
+ } else if (rr->active && ev.type == rr->evbase + RRScreenChangeNotify) {
XRRScreenChangeNotifyEvent *rre = (XRRScreenChangeNotifyEvent*)
for (screen = 0; screen < nscreens; screen++) {
if (locks[screen]->win == rre->window) {
@@ -208,7 +209,7 @@ readpw(Display *dpy, const char *pws)
}
static void
-unlockscreen(Display *dpy, Lock *lock)
+unlockscreen(Display *dpy, struct lock *lock)
{
if(dpy == NULL || lock == NULL)
return;
@@ -223,28 +224,31 @@ unlockscreen(Display *dpy, Lock *lock)
}
static void
-cleanup(Display *dpy)
+cleanup(Display **dpy, struct lock ***locks, int *nscreens)
{
int s;
- for (s = 0; s < nscreens; ++s)
- unlockscreen(dpy, locks[s]);
+ for (s = 0; s < *nscreens; ++s)
+ unlockscreen(*dpy, (*locks)[s]);
+ *nscreens = 0;
- free(locks);
- XCloseDisplay(dpy);
+ free(*locks);
+ *locks = NULL;
+ XCloseDisplay(*dpy);
+ *dpy = NULL;
}
-static Lock *
-lockscreen(Display *dpy, int screen)
+static struct lock *
+lockscreen(Display *dpy, struct xrandr *rr, int screen)
{
char curs[] = {0, 0, 0, 0, 0, 0, 0, 0};
int i, ptgrab, kbgrab;
- Lock *lock;
+ struct lock *lock;
XColor color, dummy;
XSetWindowAttributes wa;
Cursor invisible;
- if (dpy == NULL || screen < 0 || !(lock = malloc(sizeof(Lock
+ if (dpy == NULL || screen < 0 || !(lock = malloc(sizeof(struct lock
return NULL;
lock->screen = screen;
@@ -281,7 +285,7 @@ lockscreen(Display *dpy, int screen)
/* input is grabbed: we can lock the screen */
if (ptgrab == GrabSuccess && kbgrab == GrabSuccess) {
XMapRaised(dpy, lock->win);
- if (rr)
+ if (rr->active)
XRRSelectInput(dpy, lock->win, RRScreenChangeNotifyMask);
XSelectInput(dpy, lock->root, SubstructureNotifyMask);
@@ -312,13 +316,15 @@ usage(void)
int
main(int argc, char **argv) {
+ struct xrandr rr;
+ struct lock **locks;
struct passwd *pwd;
struct group *grp;
uid_t duid;
gid_t dgid;
const char *pws;
Display *dpy;
- int s, nlocks;
+ int s, nlocks, nscreens;
ARGBEGIN {
case 'v':
@@ -360,16 +366,16 @@ main(int argc, char **argv) {
die("slock: setuid: %s\n", strerror(errno));
/* check for Xrandr support */
- rr = XRRQueryExtension(dpy, , );
+ rr.active = XRRQueryExtension(dpy, , );
/* get number of screens in display "dpy" and blank them */
nscreens = ScreenCount(dpy);
- if (!(locks = calloc(nscreens, sizeof(Lock * {
+ if (!(locks = calloc(nscreens, sizeof(struct lock * {
XCloseDisplay(dpy);
die("slock: out of memory\n");
}
for (nlocks = 0, s = 0; s < nscreens; s++) {
- if ((locks[s] = lockscreen(dpy, s)) != NULL)
+ if ((locks[s] = lockscreen(dpy, , s)) != NULL)
nlocks++;
else
break;
@@ -378,7 +384,7 @@ main(int argc, char **argv) {
/* did we manage to lock everything? */
if (nlocks != nscreens) {
- cleanup(dpy);
+ cleanup(, , );
return 1;
}
@@ -386,7 +392,7 @@ main(int argc, char **argv) {
if (argc > 0) {
s
Re: [hackers] [slock] [PATCH] Properly drop privileges
On Thu, 8 Sep 2016 23:46:46 +0200 Markus Teich <markus.te...@stusta.mhn.de> wrote: Good morning, > I think its not much of a complexity to specify the group as well and > also is more explicit than implicitly using the users default group > (which might have some surprises). > > > Again I'm not really against that, just asking for some opinions. > > I've got yours! > > I don't have a clear opinion on the nogroup vs. nobody matter, but > since it is just a default and can easily be changed I just merged > the patch for now (I hope I got the correct version). If somebody[0] > gives a good argument, we'll just change the default. it is always recommended to read the motivation for doas and why sudo was invented in the first place. I wrote this patch under two assumptions - in most cases, the slock post-lock-cmd wouldn't need permissions - if it needs permissions, we can't just not drop privileges. Instead, it should be simple to just create a group "slock" or something and specify there what it is allowed to do. As an example, let's say I want to run a privileged command "rootsjob" after locking. The first thing I do is create a group "slock" and enter it in config.h. We cannot have a -u or -g flag for slock to set the dropuser because that would allow impersonation of anybody. We are protected by having it a compiled-in option because suid-applications cannot be modified without losing the suid-bit. user = "nobody" group = "slock" Now I edit my sudoers/doas.conf file and do the permission control there. Especially doas is really simple to set up and can allow fine-grained permission control. For doas.conf it would be permit nopass :slock cmd rootsjob and it can be extended from there on. slock can now be executed with slock doas rootsjob and we fixed the permission problem. The reason why doas/sudo exists is really because the duality root/non-root is too strict in many cases. The midway cannot be solved by us, but we need to put trust into the facilities provided by the system and allow the users to be flexible (for instance, one could also create a slock user, so it would be bad to implicitly set the groups from the user and vice versa). > Thanks for the contribution, FRIGN. I was happy to do it! Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [slock] [PATCH] Properly drop privileges
On Wed, 7 Sep 2016 17:48:51 +0200 Quentin Rameau <quinq@fifth.space> wrote: Hey Quentin, > Just a question though, do we need to set a group to drop privileges > to? Wouldn't getting the gid out of the user name sufficient? why cut the flexibility there? If we extract the groups from a username, we would also have to deal with supplementary groups which as a big potential to fuck things up and impose security risks. > Actually two questions, why the nogroup group instead of the nobody > group? I know that nogroup is present on OpenBSD, but the LSB suggest > the use of nobody:nobody[1] and doesn't evoke nogroup. > I don't really mind, just raising question. :) I don't know why the LSB suggests that and the LSB is a fucking mess anyway. Point is, the NFS-argument is kinda bad, given for instance the NFSv4 implementation on Linux (idmapd) also sets nobody:nogroup. It's also been the standard value for quark since forever. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [slock] [PATCH] Properly drop privileges
On Wed, 7 Sep 2016 15:25:56 +0200
FRIGN <d...@frign.de> wrote:
>
Quentin found a problem on OpenBSD which was due to the negligient use
of the passwd struct pointer. Given it points to static memory, we
ended up with a "different" struct when we went to se the privilege
drop. To fix this, we store the uid and gid directly after the sanity
check of the user and group names.
--
FRIGN <d...@frign.de>
>From ef98d1dc8075951c751c4c3bc5246bf776e88206 Mon Sep 17 00:00:00 2001
From: FRIGN <d...@frign.de>
Date: Wed, 7 Sep 2016 13:32:29 +0200
Subject: [PATCH] Ensure Polyphemus-Mitigation and properly drop privileges
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Don't hide privilege drops inside readpw() and actually make it
configurable what you are dropping to in config.h.
The privilege drop comes after opening the Display because the
user "nobody" with "nogroup" can't do that.
So why do I call this strategy the Polyphemus-Mitigation?
"""
After the giant returns in the evening and eats two more of the men,
Odysseus offers Polyphemus some strong and undiluted wine given to him
earlier on his journey. Drunk and unwary, the giant asks Odysseus his
name, promising him a guest-gift if he answers. Odysseus tells him
"Îá½ÏιÏ", which means "nobody" and Polyphemus promises to eat this
"Nobody" last of all. With that, he falls into a drunken sleep. Odysseus
had meanwhile hardened a wooden stake in the fire and now drives it into
Polyphemus' eye. When Polyphemus shouts for help from his fellow giants,
saying that "Nobody" has hurt him, they think Polyphemus is being
afflicted by divine power and recommend prayer as the answer.
"""
(source: https://en.wikipedia.org/wiki/Polyphemus)
---
config.def.h | 4
config.mk| 2 +-
slock.c | 30 +-
3 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/config.def.h b/config.def.h
index eae2d9a..6fba2b6 100644
--- a/config.def.h
+++ b/config.def.h
@@ -1,3 +1,7 @@
+/* user and group to drop privileges to */
+static const char *user = "nobody";
+static const char *group = "nogroup";
+
static const char *colorname[NUMCOLS] = {
"black", /* after initialization */
"#005577", /* during input */
diff --git a/config.mk b/config.mk
index 049305c..11357a7 100644
--- a/config.mk
+++ b/config.mk
@@ -15,7 +15,7 @@ INCS = -I. -I/usr/include -I${X11INC}
LIBS = -L/usr/lib -lc -lcrypt -L${X11LIB} -lX11 -lXext -lXrandr
# flags
-CPPFLAGS = -DVERSION=\"${VERSION}\" -DHAVE_SHADOW_H
+CPPFLAGS = -DVERSION=\"${VERSION}\" -D_DEFAULT_SOURCE -DHAVE_SHADOW_H
CFLAGS = -std=c99 -pedantic -Wall -Os ${INCS} ${CPPFLAGS}
LDFLAGS = -s ${LIBS}
COMPATSRC = explicit_bzero.c
diff --git a/slock.c b/slock.c
index da4b099..7127ebe 100644
--- a/slock.c
+++ b/slock.c
@@ -6,6 +6,7 @@
#include
#include
+#include
#include
#include
#include
@@ -83,7 +84,6 @@ dontkillme(void)
}
#endif
-/* only run as root */
static const char *
getpw(void)
{
@@ -119,10 +119,6 @@ getpw(void)
}
#endif /* HAVE_SHADOW_H */
- /* drop privileges */
- if (geteuid() == 0 &&
- ((getegid() != pw->pw_gid && setgid(pw->pw_gid) < 0) || setuid(pw->pw_uid) < 0))
- die("slock: cannot drop privileges\n");
return rval;
}
@@ -316,6 +312,10 @@ usage(void)
int
main(int argc, char **argv) {
+ struct passwd *pwd;
+ struct group *grp;
+ uid_t duid;
+ gid_t dgid;
const char *pws;
Display *dpy;
int s, nlocks;
@@ -328,6 +328,18 @@ main(int argc, char **argv) {
usage();
} ARGEND
+ /* validate drop-user and -group */
+ errno = 0;
+ if (!(pwd = getpwnam(user)))
+ die("slock: getpwnam %s: %s\n", user, errno ?
+ strerror(errno) : "user entry not found");
+ duid = pwd->pw_uid;
+ errno = 0;
+ if (!(grp = getgrnam(group)))
+ die("slock: getgrnam %s: %s\n", group, errno ?
+ strerror(errno) : "group entry not found");
+ dgid = grp->gr_gid;
+
#ifdef __linux__
dontkillme();
#endif
@@ -339,6 +351,14 @@ main(int argc, char **argv) {
if (!(dpy = XOpenDisplay(NULL)))
die("slock: cannot open display\n");
+ /* drop privileges */
+ if (setgroups(0, NULL) < 0)
+ die("slock: setgroups: %s\n", strerror(errno));
+ if (setgid(dgid) < 0)
+ die("slock: setgid: %s\n", strerror(errno));
+ if (setuid(duid) < 0)
+ die("slock: setuid: %s\n", strerror(errno));
+
/* check for Xrandr support */
rr = XRRQueryExtension(dpy, , );
--
2.7.3
Re: [hackers] [slock] [PATCH] Properly drop privileges
On Wed, 7 Sep 2016 15:17:11 +0200
FRIGN <d...@frign.de> wrote:
>
Okay, this is hopefully the last iteration.
--
FRIGN <d...@frign.de>
>From e308e34f49c89612ecdd17e989483c211453b6cb Mon Sep 17 00:00:00 2001
From: FRIGN <d...@frign.de>
Date: Wed, 7 Sep 2016 13:32:29 +0200
Subject: [PATCH] Ensure Polyphemus-Mitigation and properly drop privileges
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Don't hide privilege drops inside readpw() and actually make it
configurable what you are dropping to in config.h.
The privilege drop comes after opening the Display because the
user "nobody" with "nogroup" can't do that.
So why do I call this strategy the Polyphemus-Mitigation?
"""
After the giant returns in the evening and eats two more of the men,
Odysseus offers Polyphemus some strong and undiluted wine given to him
earlier on his journey. Drunk and unwary, the giant asks Odysseus his
name, promising him a guest-gift if he answers. Odysseus tells him
"Îá½ÏιÏ", which means "nobody" and Polyphemus promises to eat this
"Nobody" last of all. With that, he falls into a drunken sleep. Odysseus
had meanwhile hardened a wooden stake in the fire and now drives it into
Polyphemus' eye. When Polyphemus shouts for help from his fellow giants,
saying that "Nobody" has hurt him, they think Polyphemus is being
afflicted by divine power and recommend prayer as the answer.
"""
(source: https://en.wikipedia.org/wiki/Polyphemus)
---
config.def.h | 4
config.mk| 2 +-
slock.c | 26 +-
3 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/config.def.h b/config.def.h
index eae2d9a..6fba2b6 100644
--- a/config.def.h
+++ b/config.def.h
@@ -1,3 +1,7 @@
+/* user and group to drop privileges to */
+static const char *user = "nobody";
+static const char *group = "nogroup";
+
static const char *colorname[NUMCOLS] = {
"black", /* after initialization */
"#005577", /* during input */
diff --git a/config.mk b/config.mk
index 049305c..11357a7 100644
--- a/config.mk
+++ b/config.mk
@@ -15,7 +15,7 @@ INCS = -I. -I/usr/include -I${X11INC}
LIBS = -L/usr/lib -lc -lcrypt -L${X11LIB} -lX11 -lXext -lXrandr
# flags
-CPPFLAGS = -DVERSION=\"${VERSION}\" -DHAVE_SHADOW_H
+CPPFLAGS = -DVERSION=\"${VERSION}\" -D_DEFAULT_SOURCE -DHAVE_SHADOW_H
CFLAGS = -std=c99 -pedantic -Wall -Os ${INCS} ${CPPFLAGS}
LDFLAGS = -s ${LIBS}
COMPATSRC = explicit_bzero.c
diff --git a/slock.c b/slock.c
index da4b099..d297fb0 100644
--- a/slock.c
+++ b/slock.c
@@ -6,6 +6,7 @@
#include
#include
+#include
#include
#include
#include
@@ -83,7 +84,6 @@ dontkillme(void)
}
#endif
-/* only run as root */
static const char *
getpw(void)
{
@@ -119,10 +119,6 @@ getpw(void)
}
#endif /* HAVE_SHADOW_H */
- /* drop privileges */
- if (geteuid() == 0 &&
- ((getegid() != pw->pw_gid && setgid(pw->pw_gid) < 0) || setuid(pw->pw_uid) < 0))
- die("slock: cannot drop privileges\n");
return rval;
}
@@ -316,6 +312,8 @@ usage(void)
int
main(int argc, char **argv) {
+ struct passwd *pwd;
+ struct group *grp;
const char *pws;
Display *dpy;
int s, nlocks;
@@ -328,6 +326,16 @@ main(int argc, char **argv) {
usage();
} ARGEND
+ /* validate drop-user and -group */
+ errno = 0;
+ if (!(pwd = getpwnam(user)))
+ die("slock: getpwnam %s: %s\n", user, errno ?
+ strerror(errno) : "user entry not found");
+ errno = 0;
+ if (!(grp = getgrnam(group)))
+ die("slock: getgrnam %s: %s\n", group, errno ?
+ strerror(errno) : "group entry not found");
+
#ifdef __linux__
dontkillme();
#endif
@@ -339,6 +347,14 @@ main(int argc, char **argv) {
if (!(dpy = XOpenDisplay(NULL)))
die("slock: cannot open display\n");
+ /* drop privileges */
+ if (setgroups(0, NULL) < 0)
+ die("slock: setgroups: %s\n", strerror(errno));
+ if (setgid(grp->gr_gid) < 0)
+ die("slock: setgid: %s\n", strerror(errno));
+ if (setuid(pwd->pw_uid) < 0)
+ die("slock: setuid: %s\n", strerror(errno));
+
/* check for Xrandr support */
rr = XRRQueryExtension(dpy, , );
--
2.7.3
Re: [hackers] [slock] [PATCH] Properly drop privileges
On Wed, 7 Sep 2016 15:15:03 +0200
FRIGN <d...@frign.de> wrote:
>
Forget this patch, I forgot to remove my 1337 debugging line
system("id");
return 0;
from it. See attached for the fixed version. Sorry for the noise.
Cheers
FRIGN
--
FRIGN <d...@frign.de>
>From a4470c29ffde668b0b012ca379087a69b2da46fb Mon Sep 17 00:00:00 2001
From: FRIGN <d...@frign.de>
Date: Wed, 7 Sep 2016 13:32:29 +0200
Subject: [PATCH] Ensure Polyphemus-Mitigation and properly drop privileges
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Don't hide privilege drops inside readpw() and actually make it
configurable what you are dropping to in config.h.
The privilege drop comes after opening the Display because the
user "nobody" with "nogroup" can't do that.
So why do I call this strategy the Polyphemus-Mitigation?
"""
After the giant returns in the evening and eats two more of the men,
Odysseus offers Polyphemus some strong and undiluted wine given to him
earlier on his journey. Drunk and unwary, the giant asks Odysseus his
name, promising him a guest-gift if he answers. Odysseus tells him
"Îá½ÏιÏ", which means "nobody" and Polyphemus promises to eat this
"Nobody" last of all. With that, he falls into a drunken sleep. Odysseus
had meanwhile hardened a wooden stake in the fire and now drives it into
Polyphemus' eye. When Polyphemus shouts for help from his fellow giants,
saying that "Nobody" has hurt him, they think Polyphemus is being
afflicted by divine power and recommend prayer as the answer.
"""
(source: https://en.wikipedia.org/wiki/Polyphemus)
---
config.def.h | 4
config.mk| 2 +-
slock.c | 26 +-
3 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/config.def.h b/config.def.h
index eae2d9a..6fba2b6 100644
--- a/config.def.h
+++ b/config.def.h
@@ -1,3 +1,7 @@
+/* user and group to drop privileges to */
+static const char *user = "nobody";
+static const char *group = "nogroup";
+
static const char *colorname[NUMCOLS] = {
"black", /* after initialization */
"#005577", /* during input */
diff --git a/config.mk b/config.mk
index 049305c..11357a7 100644
--- a/config.mk
+++ b/config.mk
@@ -15,7 +15,7 @@ INCS = -I. -I/usr/include -I${X11INC}
LIBS = -L/usr/lib -lc -lcrypt -L${X11LIB} -lX11 -lXext -lXrandr
# flags
-CPPFLAGS = -DVERSION=\"${VERSION}\" -DHAVE_SHADOW_H
+CPPFLAGS = -DVERSION=\"${VERSION}\" -D_DEFAULT_SOURCE -DHAVE_SHADOW_H
CFLAGS = -std=c99 -pedantic -Wall -Os ${INCS} ${CPPFLAGS}
LDFLAGS = -s ${LIBS}
COMPATSRC = explicit_bzero.c
diff --git a/slock.c b/slock.c
index da4b099..70fb6a1 100644
--- a/slock.c
+++ b/slock.c
@@ -6,6 +6,7 @@
#include
#include
+#include
#include
#include
#include
@@ -83,7 +84,6 @@ dontkillme(void)
}
#endif
-/* only run as root */
static const char *
getpw(void)
{
@@ -119,10 +119,6 @@ getpw(void)
}
#endif /* HAVE_SHADOW_H */
- /* drop privileges */
- if (geteuid() == 0 &&
- ((getegid() != pw->pw_gid && setgid(pw->pw_gid) < 0) || setuid(pw->pw_uid) < 0))
- die("slock: cannot drop privileges\n");
return rval;
}
@@ -316,6 +312,8 @@ usage(void)
int
main(int argc, char **argv) {
+ struct passwd *pwd;
+ struct group *grp;
const char *pws;
Display *dpy;
int s, nlocks;
@@ -328,6 +326,16 @@ main(int argc, char **argv) {
usage();
} ARGEND
+ /* validate drop-user and -group */
+ errno = 0;
+ if (!(pwd = getpwnam(user)))
+ die("slock: getpwnam %s: %s\n", user, errno ?
+ strerror(errno) : "user entry not found");
+ errno = 0;
+ if (!(grp = getgrnam(group)))
+ die("slock: getgrnam: %s\n", group, errno ?
+ strerror(errno) : "group entry not found");
+
#ifdef __linux__
dontkillme();
#endif
@@ -339,6 +347,14 @@ main(int argc, char **argv) {
if (!(dpy = XOpenDisplay(NULL)))
die("slock: cannot open display\n");
+ /* drop privileges */
+ if (setgroups(0, NULL) < 0)
+ die("slock: setgroups: %s\n", strerror(errno));
+ if (setgid(grp->gr_gid) < 0)
+ die("slock: setgid: %s\n", strerror(errno));
+ if (setuid(pwd->pw_uid) < 0)
+ die("slock: setuid: %s\n", strerror(errno));
+
/* check for Xrandr support */
rr = XRRQueryExtension(dpy, , );
--
2.7.3
[hackers] [slock] [PATCH] Properly drop privileges
Hello fellow hackers,
I noticed a few weeks ago that slock did not drop privileges on OpenBSD
until the last commit. Now, still, the privdropping is horrible and
thus I wrote this patch to make it very strict and actually allow the
user to specifiy what to drop to and also override the supplementary
groups.
There really should not be any security compromises with a setuid
application like slock.
Cheers
FRIGN
--
FRIGN <d...@frign.de>
>From dbb170d18380a4c20c5a074e68a4f481f17d3d5e Mon Sep 17 00:00:00 2001
From: FRIGN <d...@frign.de>
Date: Wed, 7 Sep 2016 13:32:29 +0200
Subject: [PATCH] Ensure Polyphemus-Mitigation and properly drop privileges
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Don't hide privilege drops inside readpw() and actually make it
configurable what you are dropping to in config.h.
The privilege drop comes after opening the Display because the
user "nobody" with "nogroup" can't do that.
So why do I call this strategy the Polyphemus-Mitigation?
"""
After the giant returns in the evening and eats two more of the men,
Odysseus offers Polyphemus some strong and undiluted wine given to him
earlier on his journey. Drunk and unwary, the giant asks Odysseus his
name, promising him a guest-gift if he answers. Odysseus tells him
"Îá½ÏιÏ", which means "nobody" and Polyphemus promises to eat this
"Nobody" last of all. With that, he falls into a drunken sleep. Odysseus
had meanwhile hardened a wooden stake in the fire and now drives it into
Polyphemus' eye. When Polyphemus shouts for help from his fellow giants,
saying that "Nobody" has hurt him, they think Polyphemus is being
afflicted by divine power and recommend prayer as the answer.
"""
(source: https://en.wikipedia.org/wiki/Polyphemus)
---
config.def.h | 3 +++
config.mk| 2 +-
slock.c | 24 +++-
3 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/config.def.h b/config.def.h
index eae2d9a..20c1291 100644
--- a/config.def.h
+++ b/config.def.h
@@ -1,3 +1,6 @@
+static const char *user = "nobody";
+static const char *group = "nogroup";
+
static const char *colorname[NUMCOLS] = {
"black", /* after initialization */
"#005577", /* during input */
diff --git a/config.mk b/config.mk
index 049305c..11357a7 100644
--- a/config.mk
+++ b/config.mk
@@ -15,7 +15,7 @@ INCS = -I. -I/usr/include -I${X11INC}
LIBS = -L/usr/lib -lc -lcrypt -L${X11LIB} -lX11 -lXext -lXrandr
# flags
-CPPFLAGS = -DVERSION=\"${VERSION}\" -DHAVE_SHADOW_H
+CPPFLAGS = -DVERSION=\"${VERSION}\" -D_DEFAULT_SOURCE -DHAVE_SHADOW_H
CFLAGS = -std=c99 -pedantic -Wall -Os ${INCS} ${CPPFLAGS}
LDFLAGS = -s ${LIBS}
COMPATSRC = explicit_bzero.c
diff --git a/slock.c b/slock.c
index da4b099..9d4086d 100644
--- a/slock.c
+++ b/slock.c
@@ -6,6 +6,7 @@
#include
#include
+#include
#include
#include
#include
@@ -83,7 +84,6 @@ dontkillme(void)
}
#endif
-/* only run as root */
static const char *
getpw(void)
{
@@ -119,10 +119,6 @@ getpw(void)
}
#endif /* HAVE_SHADOW_H */
- /* drop privileges */
- if (geteuid() == 0 &&
- ((getegid() != pw->pw_gid && setgid(pw->pw_gid) < 0) || setuid(pw->pw_uid) < 0))
- die("slock: cannot drop privileges\n");
return rval;
}
@@ -316,6 +312,8 @@ usage(void)
int
main(int argc, char **argv) {
+ struct passwd *pwd;
+ struct group *grp;
const char *pws;
Display *dpy;
int s, nlocks;
@@ -328,6 +326,14 @@ main(int argc, char **argv) {
usage();
} ARGEND
+ /* validate drop-user and -group */
+ errno = 0;
+ if (!(pwd = getpwnam(user)))
+ die("slock: getpwnam: %s\n", strerror(errno));
+ errno = 0;
+ if (!(grp = getgrnam(group)))
+ die("slock: getgrnam: %s\n", strerror(errno));
+
#ifdef __linux__
dontkillme();
#endif
@@ -339,6 +345,14 @@ main(int argc, char **argv) {
if (!(dpy = XOpenDisplay(NULL)))
die("slock: cannot open display\n");
+ /* drop privileges */
+ if (setgroups(1, &(grp->gr_gid)) < 0)
+ die("slock: setgroups: %s\n", strerror(errno));
+ if (setgid(grp->gr_gid) < 0)
+ die("slock: setgid: %s\n", strerror(errno));
+ if (setuid(pwd->pw_uid) < 0)
+ die("slock: setuid: %s\n", strerror(errno));
+
/* check for Xrandr support */
rr = XRRQueryExtension(dpy, , );
--
2.7.3
Re: [hackers] [slock] Unify how we check passwords between different OSes
On Tue, 6 Sep 2016 20:52:05 +0200 Quentin Rameau <quinq@fifth.space> wrote: > I second this patch. As a side-note, this patch is necessary and gives the ground work for a patch by me that drops privileges after the getpw function. Previously on OpenBSD, slock would _never_ drop privileges, which is not a very sane thing to do. If this patch is applied, I'll send my patch for slock as well to do the privilege drop properly. Currently, the privilege drop on Linux does not reset supplementary groups; this is adressed in the upcoming patch as well. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [quark] Use sizeof() instead of magic constants || FRIGN
On Mon, 5 Sep 2016 10:05:16 +0200 FRIGN <d...@frign.de> wrote: > [...], but I respect the per-process style. Of course I meant "per-project style". -- FRIGN <d...@frign.de>
Re: [hackers] [quark] Use sizeof() instead of magic constants || FRIGN
On Mon, 5 Sep 2016 09:53:57 +0200 Anselm R Garbe <garb...@gmail.com> wrote: Hey Anselm, > That's wrong. There are good reasons like forward declarations / > opaque type definitions that incorporate typedef. how are you using the term forward declaration in this context? Are you referring to function prototypes with "simpler" arguments? As to opaque type definitions: There are very few cases where this actually makes sense, especially in a language like C. Opaque types often "present" themselves when developing toolboxes for numerical issues, but often the incentives are not too high. In dwm/dmenu the convention is to have the first type-letter uppercase, like "Display". This is imho a good naming convention in itself and does not require a new "format" for sizeof without parentheses if you are so inclined to use typedefs for your structs. I don't see a good reason to use typedefs in dwm/dmenu in the first place, but I respect the per-process style. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [quark] Use sizeof() instead of magic constants || FRIGN
On Mon, 5 Sep 2016 09:55:49 +0200
Anselm R Garbe <garb...@gmail.com> wrote:
Hey Anselm,
> Beware! There have been zealots arguing that using goto is a bad
> practice for similar reasons.
>
> typedef's have to be considered more carefully.
zealots argue against goto because it's a low-level feature and they
have not understood that loops are basically using goto under the hood.
So we can identify the notion against goto as the fear of low-level
stuff and a fear of low-abstraction.
With typedefs, we are going another direction, because typedefs are a
"high-abstraction" just like using
if () {
} else if () {
if () {
} else {
}
}
trees just not to have gotos in the code.
Cheers
FRIGN
--
FRIGN <d...@frign.de>
Re: [hackers] [quark] Use sizeof() instead of magic constants || FRIGN
On Mon, 5 Sep 2016 09:42:47 +0200
Anselm R Garbe <garb...@gmail.com> wrote:
Hey Anselm,
> Why is typedef'ing structs bad practice?
because there's no reason for it other than syntax candy. It also hides
from the user what he is dealing with and when I want to lookup a
struct definition I often have to jungle-jump across multiple typedef
layers to finally reach the definition.
If I have a program for handling bank accounts and define an
account-struct like
struct account {
char *firstname;
char *lastname;
int balance;
};
I think it's more descriptive in the code to declare a local variable
like
struct account fisher;
rather than having a typdef
#typedef struct account Account
which yields
Account fisher;
It may look "nicer", "cleaner", "shorter", but it just hides
information from the user. Look at the Posix socket interfaces; there's
a reason why all the sock-address structs were left as-is and why a
messy project like X11 typedefs the shit out of its structs. The
ultimate reason is because X11 has become so complex that its authors
tried to at least make it visually half-appealing by "shortening" their
code with struct typedefs. If your code has become so complex and
overwhelming that plain "struct structname" constructs can't be fitted
in there any more you should fix it. That's my opinion.
Cheers
FRIGN
--
FRIGN <d...@frign.de>
Re: [hackers] [quark] Use sizeof() instead of magic constants || FRIGN
On Mon, 5 Sep 2016 09:39:24 +0200 Anselm R Garbe <garb...@gmail.com> wrote: Hey Anselm, > Why should I try. This is blatantly obvious wrong code. > > I'm referring to NOT using parentheses to make it explicitely clear, > that the expression is not referring to a type. See sec. 6.5.3 in the > C lang spec for further details. why would there ever be a doubt that "unsigned char" is a type? Even if you use the stdint.h types, e.g. uint8_t, Posix has noticed and taken measures to make it clear with the _t suffix that uint8_t is a type. As I mentioned earlier, if you cannot discern a type name from a variable name it's your problem and should not be "fixed" by having a strange use of sizeof which is error-prone, as Ali has well shown. This obviously goes the other way around too. One should not use variable names that can be misinterpreted as a typename. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [quark] Use sizeof() instead of magic constants || FRIGN
On Mon, 5 Sep 2016 09:37:17 +0200 Anselm R Garbe <garb...@gmail.com> wrote: Hey Anselm, > I disagree. Not using parentheses as sizeof arguments makes it pretty > clear, that the expression is *not* about the size of a type, but > rather needs to be evaluated. In dwm/dmenu code this has been > respected until now for a very long time. I would keep this principle > in place for other suckless tools as well. but this only happens if you typedef your structs, which I think is bad practice. If you do a #typedef struct hw homework this is your own fault. Using sizeof(struct hw) shows clearly it's a type, whereas sizeof(homework) is not clear. But this is more a criticism of extreme typedeffing. Stop using typedefs for structs and you won't have this problem any more. If you can't discern type names from variable names, this is your problem. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [quark] Use sizeof() instead of magic constants || FRIGN
On Mon, 5 Sep 2016 07:42:36 +0200 Anselm R Garbe <garb...@gmail.com> wrote: Hey Anselm, > Quick note: your syntax usage of sizeof is not 100% accurate. > > Use 'sizeof(type)' with brackets but 'sizeof var' without. I use sizeof always function-like and see no reason why I shouldn't use "sizeof(var)" and instead use "sizeof var". It doesn't alter the code behaviour and ultimately it's all about readability. If you eyes have become accustomed to always use function-like syntax for such operators the best bet is not to break this style because it's not "necessary" to have parentheses for variable sizeof's. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [quark] missing newline
On Fri, 02 Sep 2016 17:26:12 +0300 "Ali H. Fardan" <r...@firemail.cc> wrote: > sorry, wrong patch Thanks! :) http://git.suckless.org/quark/commit/?id=12ebbc5dedfb212af3b93bb1762aec31bb59562f -- FRIGN <d...@frign.de>
Re: [hackers] [sbase] a question regarding date(1)
On Thu, 01 Sep 2016 19:01:50 +0300 "Ali H. Fardan" <r...@firemail.cc> wrote: Hey Ali, > so I just came across this if (argc) statement and > I'm not sure why it is placed there in the first > pleace, I am assuming that argc is not tinkered with > (via arg.h), but the program works well without it, > so my question is, is this if() statement incorrect > or it's just there because of arg.h? arg.h tinkers with argc so that after ARGEND argc represents the number of arguments. Thus the "if (argc)" means "if we have any arguments". $ tool -f farg arg when the flag f takes a mandatory argument farg the resulting argc for instance is 1 (and *argv points at "arg"). I hope that explains things. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [slock] Fix my previous commit and some light refactoring
On Thu, 1 Sep 2016 15:05:09 +0200 Markus Teich <markus.te...@stusta.mhn.de> wrote: Hey Markus, > I don't think a config.h variable is the best option. When starting > slock from the terminal the problem does not occur, so you don't want > a timeout in this case. For key bindings it's easy to just pass a > command line flag. Therefore I favor a shell wrapper or the direct > command line flag, while I think the direct flag approach is best, > since it's job (waiting) can be done from C as easily as it would be > in a shell wrapper, but it avoid one extra layer of complexity. we are discussing an edge-case here. Retrying for 600ms is after a lot of consideration definitely the best way. I also checked the possibility of force ungrabbing the grab by other applications using XF86Ungrab, but it doesn't work. If it had worked, a possibility would've been to wait indefinitely after forcefully ungrabbing input; so we just wait for the user to release his keys (if it takes 2 seconds or whatever, say Mr. Plinkett uses his Linux box :P). Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [slock] Fix my previous commit and some light refactoring
On Thu, 1 Sep 2016 13:45:24 +0200 Quentin Rameau <quinq@fifth.space> wrote: Hey Quentin, > My last commit removing the input grabbing waiting loop was a mistake, > I tested it on a specific setup which didn't raise any error while > regular setups should do. > So the second of the four next commits fixes that. > The other three try to move duplicated behaviour inside unique > functions and move global variables inside functions which > exclusively need them. Patches following. the patches look good to me and I would favor them to be merged. Thanks for your work, Quentin! Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [slock] [PATCH] simplify fix for CVE-2016-6866
On Wed, 31 Aug 2016 19:33:41 +0200 Markus Teich <markus.te...@stusta.mhn.de> wrote: Hey Marcus, > after reading Erics original CVE report and proposed fix again and > thinking about it, I came to the conclusion that this is a cleaner > fix. It calls crypt() pre-locking with a bogus "x" as password just > to see if the pws value is correct and other system requirements are > met to call crypt later on after the password has been entered. > > I will apply it tomorrow if there are no objections. are you sure we are not hitting any TOCTTOU problems here? Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [slock] Exit as soon as possible on input grabbing error
On Tue, 30 Aug 2016 19:12:26 +0200 Quentin Rameau <quinq@fifth.space> wrote: Hey Quentin, > Personally I don't mind, but you could break the lines after each > argument and satisfy both churches. ;) > ie: > + fprintf(stderr, > + "slock: unable to grab mouse pointer for > screen %s\n", > + screen); > … > + fprintf(stderr, > + "slock: unable to grab keyboard for screen > %s\n", > + screen); this looks like a good compromise. :) Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [slock] Exit as soon as possible on input grabbing error
On Tue, 30 Aug 2016 17:33:09 +0200 Quentin Rameau <quinq@fifth.space> wrote: > We want to know at once if slock failed or not to lock the screen, not > seing a black screen for a whole second (or two) and then die. > Thanks to ^7heo for reporting this. I support this patch. We intensely discussed this on IRC and it is the only sane way to make sure the behaviour is proper. Timing heuristics are never good. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [slock] [PATCH] Refactor main()
On Mon, 22 Aug 2016 18:55:40 +0200
Quentin Rameau <quinq@fifth.space> wrote:
Hey Quentin,
> I agree with Markus remarks and I have a few more myself:
>
> > + ARGBEGIN {
> > + case 'v':
> > + fprintf(stderr, "slock-"VERSION"\n");
>
> Do we really need that?
look at dwm and st, it makes sense to have a v-flag. I was not very
positive towards it a few months ago, but have changed my mind,
especially if you help debugging problems for a not-so-experienced user
and quickly want to find out the version he is running.
> > - XSync(dpy, False);
> > + XSync(dpy, 0);
>
> Xlib provides and use a boolean type for its functions, I think we
> should stick to that.
Look at /usr/include/X11/Xlib.h:
#define Bool int
#define Status int
#define True 1
#define False 0
I can't believe we still put up with this madness! And I won't even
mention here the fact the Xlib-devs probably never heard of typedefs
before.
The sooner we get rid of this pseudo-boolshit the better. I wanted to
do it wrt slock function-by-function, but I can also make a separate
unboolify-patch.
> > free(locks);
> ^- Is it necessary?
> > XCloseDisplay(dpy);
> [...]
> > + free(locks);
> Same remark. -^
> > + XCloseDisplay(dpy);
> > + die("slock: fork failed: %s\n", strerror
> [...]
> > free(locks);
> ^- Same remark.
> > XCloseDisplay(dpy);
It doesn't look like it. It is more of a note to maybe investigate even
more if we need to release the locks in error cases.
The libc will of course handle all the freeing on exit for us, same
with file descriptors. The cleanup is done at the end anyway, so I
thought I add it in places where we also return prematurely to make it
possible in the future to spot the places where we might have to
release locks manually. The X-Server is pretty strange in many aspects
and the entire story has not been told yet. I wanted to leave this open
until later when I refactored the unlockscreen() function.
Cheers
FRIGN
--
FRIGN <d...@frign.de>
Re: [hackers] [slock] [PATCH] Refactor main()
On Mon, 22 Aug 2016 19:34:24 +0300 "Ali H. Fardan" <r...@firemail.cc> wrote: Hey Ali, > It's their fault, and if it was a mistake, people will notice it in > the patch before anyone proceeds to applying it. yeah, you're right. This requires strict patch review, and I'm confident that this is given here on our mailing list (as we can also observe with my patch). :) I corrected it accordingly, as you probably have already seen. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [slock] [PATCH] Refactor main()
On Mon, 22 Aug 2016 12:32:28 +0200
Markus Teich <markus.te...@stusta.mhn.de> wrote:
Hey Markus,
> thanks for the update. Apart from the points mentioned below, the
> changes look good.
>
> > - if (!getpwuid(getuid()))
> > - die("no passwd entry for you\n");
> > + /* Check if the current user has a password entry */
> > + errno = 0;
> > + if (!getpwuid(getuid())) {
> > + if (errno == 0) {
> > + die("slock: no password entry for current
> > user\n");
> > + } else {
> > + die("slock: getpwuid: %s\n", strerror
> > (errno));
> > + }
> > + }
>
> According to the coding style the inner if should not have braces. If
> you want to change the coding style, for consistencys sake please
> start a general discussion about it before introducing your
> preference in patches.
ah, that might have slipped past. I generally code according to the
suckless.org coding styles[0], and given there was no reference on how
the project specific block looks like, I did it that way.
For one-line if-statements it makes sense to keep the braces away, but
big if-else-blocks benefit from braces in my opinion.
> > + /* run post-lock command */
> > + if (argc > 0) {
> > + switch(fork()) {
>
> I think you want a space after the `switch`.
Oh yeah, good catch!
> > diff --git a/util.h b/util.h
> > index 6f748b8..4f170a2 100644
> > --- a/util.h
> > +++ b/util.h
> > @@ -1,2 +1,6 @@
> > +#include "arg.h"
> > +
> > +extern char *argv0;
> > +
> > #undef explicit_bzero
> > void explicit_bzero(void *, size_t);
>
> Why do we need that level of inderection? We can just include arg.h
> from slock.c directly.
Yes we could. I can change this, but was a bit irritated that util.h is
so empty.
Cheers
FRIGN
--
FRIGN <d...@frign.de>
Re: [hackers] [slock] [PATCH] Refactor main()
On Mon, 22 Aug 2016 10:59:16 +0200 Anselm R Garbe <garb...@gmail.com> wrote: > I prefer it this way. So is it good to merge? Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [slock] [PATCH] Refactor main()
On Mon, 22 Aug 2016 09:30:29 +0200
Anselm R Garbe <garb...@gmail.com> wrote:
Hey Anselm,
> First, why using
>
> /*
> * tiny comment
> */
>
> instead of
> /* tiny comment */
that is definitely a style matter, but it serves to provide a hierarchy
for comments. Of course, if you wish, I can change this accordingly.
> Second, why introducing superfluous scope brackets?
>
> I prefer
>
> if (!(dpy = XOpenDisplay(0)))
> die("cannot open display\n");
>
> to cumbersome:
>
> if (!(dpy = XOpenDisplay(NULL))) {
> die("slock: cannot open display\n");
> }
It saves one line to leave the brackets out, but it has happened to me
more than once that people add a line to the if clause not paying
attention and effectively introducing a bug. And FWIW, in my opinion,
having brackets improves readability a bit.
But again, if it is your explicit desire, I can create a version of the
patch with the brackets removed.
Cheers
FRIGN
--
FRIGN <d...@frign.de>
[hackers] [slock] [PATCH] Refactor main()
Good morning fellow hackers,
after discussing this with Markus we agreed that it is time for slock
to have a refactor so it benefits from all the best practices(tm) we
learned about in the last few years and worked out working on sbase and
other projects.
In the long run the target is to go through each function and very
strictly do error checks where necessary and also reduce global state
where it might be a problem. Given slock is suid, we don't want to take
any risks and provide as little attack surface as possible.
Besides stricter error checking this patch also improves the usage and
makes the code more readable by pulling in arg.h, which has proven to
be very useful in the last few years.
Please provide feedback on this patch and test it.
Cheers
FRIGN
--
FRIGN <d...@frign.de>
>From 3c8552d65a0cd999e0f485669e5c99ea7c3399a4 Mon Sep 17 00:00:00 2001
From: FRIGN <d...@frign.de>
Date: Mon, 22 Aug 2016 00:25:21 +0200
Subject: [PATCH] Refactor main()
- Add arg.h and fix usage
Given slock is suid we don't want to have half-measures in place to
parse the arguments in case the code is changed in the future with
somebody not paying enough attention.
Also, fix the usage string output to be more consistent across the
suckless toolbase and make it reflect the manpage entry.
- Comments
Use proper block comments and add/change them where necessary
to help in studying the code.
- Error messages
Consistently prepend them with "slock:" and fix wording and
do a proper cleanup before quitting (XCloseDisplay and free
the locks), making the die() semantics consistent with st's.
- getpwuid() error reporting
Properly present an error message if getpwuid() fails.
- fork() error reporting
Properly present an error message if fork() fails. If we cannot
close the connection within the fork context we abort the
operation and report an error.
- execvp() error handling
If execvp fails, we cannot call die() afterwards as this implies
calling exit(). We must use _exit() to prevent the libc from
doing now "illegal" cleanup-work.
---
arg.h | 65 +
slock.c | 101
util.h | 4 +++
3 files changed, 139 insertions(+), 31 deletions(-)
create mode 100644 arg.h
diff --git a/arg.h b/arg.h
new file mode 100644
index 000..0b23c53
--- /dev/null
+++ b/arg.h
@@ -0,0 +1,65 @@
+/*
+ * Copy me if you can.
+ * by 20h
+ */
+
+#ifndef ARG_H__
+#define ARG_H__
+
+extern char *argv0;
+
+/* use main(int argc, char *argv[]) */
+#define ARGBEGIN for (argv0 = *argv, argv++, argc--;\
+ argv[0] && argv[0][0] == '-'\
+ && argv[0][1];\
+ argc--, argv++) {\
+char argc_;\
+char **argv_;\
+int brk_;\
+if (argv[0][1] == '-' && argv[0][2] == '\0') {\
+ argv++;\
+ argc--;\
+ break;\
+}\
+for (brk_ = 0, argv[0]++, argv_ = argv;\
+ argv[0][0] && !brk_;\
+ argv[0]++) {\
+ if (argv_ != argv)\
+ break;\
+ argc_ = argv[0][0];\
+ switch (argc_)
+
+/* Handles obsolete -NUM syntax */
+#define ARGNUMcase '0':\
+ case '1':\
+ case '2':\
+ case '3':\
+ case '4':\
+ case '5':\
+ case '6':\
+ case '7':\
+ case '8':\
+ case '9'
+
+#define ARGEND }\
+ }
+
+#define ARGC() argc_
+
+#define ARGNUMF() (brk_ = 1, estrtonum(argv[0], 0, INT_MAX))
+
+#define EARGF(x) ((argv[0][1] == '\0' && argv[1] == NULL)?\
+((x), abort(), (char *)0) :\
+(brk_ = 1, (argv[0][1] != '\0')?\
+ ([0][1]) :\
+ (argc--, argv++, argv[0])))
+
+#define ARGF() ((argv[0][1] == '\0' && argv[1] == NULL)?\
+(char *)0 :\
+(brk_ = 1, (argv[0][1] != '\0')?\
+ ([0][1]) :\
+ (argc--, argv++, argv[0])))
+
+#define LNGARG() [0][0]
+
+#endif
diff --git a/slock.c b/slock.c
index a00fbb9..4230fe2 100644
--- a/slock.c
+++ b/slock.c
@@ -25,6 +25,8 @@
#include "util.h"
+char *argv0;
+
enum {
INIT,
INPUT,
@@ -54,7 +56,6 @@ die(const char *errstr, ...)
{
va_list ap;
- fputs("slock: ", stderr);
va_start(ap, errstr);
vfprintf(stderr, errstr, ap);
va_end(ap);
@@ -280,8 +281,7 @@ lockscreen(Display *dpy, int screen)
static void
usage(void)
{
- fprintf(stderr, "usage: slock [-v|POST_LOCK_CMD]\n");
- exit(1);
+ die("usage: slock [-v | cmd [arg ...]]\n");
}
int
@@ -290,64 +290,103 @@ main(int argc, char **argv) {
const char *pws;
#endif
Display *dpy;
- int screen;
-
- if ((argc >= 2) && !strcmp("-v", argv[1]))
- die("version %s, © 2006-2016 slock engineers\n", VERSION);
+ int s, nlocks;
- /* treat first argument starting with a '-' as option */
- if ((argc >= 2) && argv[1][0] == '-')
+ ARGBEGIN {
+ case 'v':
+ fprintf(stderr, "slock-"VERSION"\n");
+ return 0;
+ default:
usage();
+ } ARGEND
#ifdef __linux__
do
Re: [hackers] [sent] Fix error-messages || FRIGN
On Fri, 12 Aug 2016 11:18:43 +0200 Markus Teich <markus.te...@stusta.mhn.de> wrote: Hey Markus, > Pro always: Saves two characters in each call. > Pro never: Same behaviour as the printf-family from libc. > > I am pro always due to convenience. I agree. I sometimes forget the \n and it's easy to miss given some errors rarely happen. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [sxiv] Discussion
On Sun, 7 Aug 2016 23:33:58 +0100 Dimitris Papastamos <s...@2f30.org> wrote: > hackers@ is not the right place for this kind of discussion. My bad, I'll repost this on dev@. -- FRIGN <d...@frign.de>
Re: [hackers] Re: [slock][patch] clear passwords with explicit_bzero
On Sun, 7 Aug 2016 13:03:48 +0200 Hiltjo Posthuma <hil...@codemadness.org> wrote: > Can someone apply this patch or give a reason why it sucks? :) I second this patch. -- FRIGN <d...@frign.de>
[hackers] [sxiv] Discussion
Hello fellow hackers, don't take it personally, Bert, but I don't think your project sxiv[0] belongs to the suckless git-repository. Not only is it licensed with the GPLv2, which is despicable in itself, but the code doesn't even look suckless to me and there are good ways to go around the whole image-format-cancer-spread nowadays. Look at how sent does image handling; it's definitely best to push this task toward other tools, like farbfeld, for easy internal handling. C: 3663 (98.05%) sh: 43 (1.15%) awk: 30 (0.80%) Do we really need a project the size of dwm to display images? The name suckless stands for quality software, which foremost tries to accomplish elegance and simplicity. There are already too many git repositories in git.suckless.org, and added to this it seems the sxiv repo on git.suckless.org is just a github mirror, with all its implied beauty[1]. Do I really need to dig around github now to see what the commit fixed? What do you think? Cheers FRIGN [0]:http://git.suckless.org/sxiv/tree/ [1]:http://git.suckless.org/sxiv/commit/?id=53a72c7b657d9dc3347d9d68e0b9a00773efe732 -- FRIGN <d...@frign.de>
Re: [hackers] [dmenu] Print highlighted input text only on single match || Quentin Rameau
On Tue, 26 Jul 2016 22:52:16 +0200 (CEST) g...@suckless.org wrote: > Print highlighted input text only on single match You probably merged the wrong branch here mate. -- FRIGN <d...@frign.de>
[hackers] [sent] add ff and ff.bz2 support in config.def.h
Hello everybody,
I wrote a patch as an addition to config.def.h to support ff and ff.bz2
images. Let me know what you think.
Cheers
FRIGN
--
FRIGN <d...@frign.de>
>From c8504ac1a1f1f508fc0989bdc8d0a59c8efd2d98 Mon Sep 17 00:00:00 2001
From: FRIGN <d...@frign.de>
Date: Thu, 14 Jul 2016 11:19:36 +0200
Subject: [PATCH] Add .ff and .ff.bz2 to filter rules
So you can use them directly :)
---
config.def.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/config.def.h b/config.def.h
index 92d577c..eac96db 100644
--- a/config.def.h
+++ b/config.def.h
@@ -47,5 +47,7 @@ static Shortcut shortcuts[] = {
};
static Filter filters[] = {
- { "\\.(png|jpg|gif)$", "2ff" },
+ { "\\.ff$", "cat" },
+ { "\\.ff.bz2$", "bunzip2" },
+ { "\\.[a-z0-9]+$", "2ff" },
};
--
2.4.10
Re: [hackers] [farbfeld] Add support for .bz2 to 2ff
On Thu, 14 Jul 2016 10:38:05 +0200
FRIGN <d...@frign.de> wrote:
> { "\\.ff.bz2$", "bunzip | 2ff" },
my bad, it has to be
{ "\\.ff.bz2$", "bunzip" },
--
FRIGN <d...@frign.de>
Re: [hackers] [farbfeld] Add support for .bz2 to 2ff
On Thu, 14 Jul 2016 09:55:07 +0200
Willy Goiffon <wi...@mailoo.org> wrote:
Hey Willy,
> If figured it would be great to have `2ff` support bzip2 compressed
> farbfeld images, in order to start promoting this format.
> Attached is a patch adding support for bzip2 compressed files to
> `2ff`.
thanks for your patch. At first I was worried that it might expand
the scope of 2ff too much, but it really is no problem.
However, I'm worry about other people approaching with questions to
also add lzma and other formats for completeness.
You probably had sent[0] in mind when you wrote this patch, but
looking at the filter rules
static Filter filters[] = {
{ "\\.(png|jpg|gif)$", "2ff" },
};
it is easy to add
static Filter filters[] = {
{ "\\.ff.bz2$", "bunzip | 2ff" },
};
with the same effect.
Can you give me a good reason or context example where adding the
compression support to 2ff proves to be useful?
Cheers
FRIGN
[0]: http://git.suckless.org/sent
--
FRIGN <d...@frign.de>
Re: [hackers] [dwm][PATCH] gaps
On Sun, 10 Jul 2016 23:33:24 +0200 Matej Focko <matej.fo...@gmail.com> wrote: > --- > config.def.h | 1 + > dwm.c| 16 +--- > 2 files changed, 10 insertions(+), 7 deletions(-) No description, not according to the coding style? Spaces instead of tabs? Are you bloody serious? -- FRIGN <d...@frign.de>
Re: [hackers] [sbase][PATCH] printf: support escaping '%' with '%%'. See printf(1p) EXAMPLES section.
On Sat, 25 Jun 2016 19:11:09 +0200
pranomostro <pranomes...@gmail.com> wrote:
Hey Pranomostro,
nice catch! Care to send it as a git-patch?
Cheers
FRIGN
> ---
> printf.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/printf.c b/printf.c
> index 2e24817..7bf6fe5 100644
> --- a/printf.c
> +++ b/printf.c
> @@ -103,8 +103,10 @@ main(int argc, char *argv[])
> arg = "";
> cooldown = 1;
> }
> - } else
> + } else {
> putchar('%');
> + continue;
> + }
>
> switch (format[i]) {
> case 'b':
> --
> 2.9.0
--
FRIGN <d...@frign.de>
Re: [hackers] [swerc] local suckless.org copyright notice || Anselm R Garbe
On Mon, 13 Jun 2016 14:17:01 +0200 (CEST) g...@suckless.org wrote: > - 2006-2013 suckless.org community | href="http://garbe.us/Contact;>Impressum > + 2006-2015 suckless.org community | href="http://garbe.us/Contact;>Impressum Isn't it 2016? -- FRIGN <d...@frign.de>
Re: [hackers] [scc] Revert "[driver] use pointers in tools lookup table" || Quentin Rameau
On Tue, 7 Jun 2016 16:42:13 +0200 Qentin Rameau <quinq@fifth.space> wrote: Hey Quentin, > Why would I have? > scc is the driver, it's not bound to cc1 or cc2. if you look at the type-translation-tables, there is kind of a "hack" to support the Plan 9 compiler as well. The idiom of making a pointer struct-array was circumvented to avoid doing that. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [scc] Revert "[driver] use pointers in tools lookup table" || Quentin Rameau
On Tue, 7 Jun 2016 15:22:13 +0200 (CEST) g...@suckless.org wrote: Hey Quentin, > Revert "[driver] use pointers in tools lookup table" > > This reverts commit 86e6d58d2e1059bb493df922bc89a1cc2b92ee83. > > This was a leftover from a test I forgot to drop before pushing. > Sorry for the noise. I was already wondering. If you had applied that change, you would've also had to make the changes in the arch-code-lookup-tables. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [dwm] [PATCH] Replace str[n]cpy with strlcpy
On Mon, 6 Jun 2016 20:12:19 +0200
k...@shike2.com wrote:
Hey Roberto,
> What happens if you pass an incorrect size to strlcpy?. Please,
> stop of saying stupid things.
well, you don't pass invalid sizes to strlcpy. It's like if I
asked: Robert, what happens if I pass an invalid path to
unlink()? Huh, see, your world is crumbling into pieces.
> if (strlcpy(dst, src, nsrc) >= nsrc)
> error();
>
> is equal to:
>
> if (nsrc >= ndst)
> error();
> memcpy(dst, src, nsrc);
Nope, it's not. Keep in mind strlcpy fills the rest
of the memory area with 0's.
Also, I suspect you have not understood strlcpy()
at all. Read the manpage!
The size-argument of strlcpy is wrt to the size of
the _destination_, not the _source_.
Your example also hides the fact that "nsrc" has to
be first determined with strlen().
> but the code with strlcpy is slower and not portable.
I give you a beer at the next slcon if you can show me
a solution which does the same as strlcpy and is faster.
> There is a reason why after 16 years strlcpy is not in any
> standard, no C11, no POSIX, and it is because it sucks a lot.
Well, this is up for debate. I respect your opinion though.
> From my point of view the worst thing is that people believe
> that using strlcpy the code magically becomes secure, and this
> is a totally false security sensation. You have to check the
> return code, and it means that the code is totally equivalent
> to an explicit if. Look for example this case:
>
> deluser(strlcpy(dst, "user15", 4));
>
> Since you are not checking any return code there you are not
> deleting the correct user, and this kind of attacks can be very
> easy of attack, more easier than stack overflow.
Yes, this is a very good point. However, if you used strncpy,
the string would not even be terminated in case of an overflow.
It's funny to see how people wage holy war between strlcpy and
strncpy only to realize that of the two strlcpy is the safer
one.
Now, of course, you have to check the return value.
> In a previous mail you said that one of the reasons of using
> strlcpy was to avoid problems in the future due to modifications
> in the code. Did you think about it before writing it?. You
> can say that of _ANY_ operation in C, mainly with pointers and
> indexes, but strlcpy can not help at all in a situation like this:
>
>
> #define LENA 5
> #define LENB 6
>
> char sa[LENB];
>
> f(sa);
>
> f(char s[LENB])
> {
> strlcpy(s, "This is a very long string", LENB);
> }
>
> and now you have this patch:
>
> - char sa[LENB];
> + char sa[LENA];
>
> Do you see? strlcpy didn't help at all, and due to the
> false security sensation the programmer didn't dig to
> see all the side effects of changing the size of sa.
> C is a very low level language, and it is a language
> without support for strings, and the only way of writting
> correct code is to be very carefull and before doing any
> change check everything, and look for all the possible
> errors due to your change.
You cannot possibly check these things by yourself. What
strlcpy does is give you an insurance in case there is an
overflow that the program doesn't pass pointers to
unterminated strings around. Of course you have to check
the return value anyway.
I don't know why you are always riding the same horse
here given that:
1) dwm used strcpy (!)
2) dwm used _unchecked_ strncpy (!)
so you definitely should calm down a bit here.
> And of course, strlcpy is also totally useless because
> you can do the same work with snprintf.
I'm not sure if people want to involve the complex
mechanics of the *printf functions to do simple string
copying. You have to agree that on a decision between
strncpy and strlcpy, strncpy wins, no matter the context.
Of course, to use strlcpy safely, you have to check the
return code, because truncation is also yielding garbage.
However, at least this garbage won't blow up in your
face.
> PD: I don't want to begin a flame war, but please, stop
> of being a fan boy and think for yourself, try to find
> the strong points and what is propaganda.
Maybe you should evaluate your position a bit better
before going all grandpa here on this ml.
Your first code example is embarassing and shows that
despite your high skill in C you seem to be fighting
the wrong war here.
Cheers
FRIGN
--
FRIGN <d...@frign.de>
Re: [hackers] [dwm] [PATCH] Replace str[n]cpy with strlcpy
On Fri, 3 Jun 2016 17:58:12 +0200 k...@shike2.com wrote: Hey Roberto, > safe?. You are not checking any of the return codes. You are only > moving the problem from some place to another place. Please add > checks and stop using non portable functions. I don't want your > shit, thanks. there is also another point here: strlcpy is safer than strcpy and strncpy because _if_ there is an overflow the string will be 0-terminated. I'm not sure if there even should be an error-out in case for instance we overflow writing the "broken"-state-string to a client-name. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [dwm] [PATCH] Replace str[n]cpy with strlcpy
On Fri, 3 Jun 2016 17:58:12 +0200 k...@shike2.com wrote: Hey k0ga, > safe?. You are not checking any of the return codes. You are only > moving the problem from some place to another place. Please add > checks and stop using non portable functions. I don't want your > shit, thanks. I just saw that I forgot to amend this part of the patch to this one here. Will push it later, like the following: if (strlcpy(..., size) >= size) ... The only reason why strlcpy is "non-portable" is because the Posix-committee has sticks up their asses (same with the glibc guys). Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [libzahl] Switch to ISC license. || Mattias Andrée
On Thu, 2 Jun 2016 22:01:47 +0200 Mattias Andrée <maand...@kth.se> wrote: Hey Mattias, > Do you think anyone have choked on pastries before and sued? > Do most countries actually require that you state that > your are not responsible for damages causes by something > you are warning about? The warning is enough here, or at > least I have never seen a disclaimer, only a warning label, > and we don't even have translation for ‘disclaimer’. it's not about countries, but commercial law. If you hit a judge who is not very smart when it comes to computers you can have a bad day and actually get sued for such damages. Why take the risk? > Nice one, but I think the FreeBSD project does too much > GPL-bashing. GPL is a good license, at least if you value > free software higher than open source. GPL is not about freedom, it's about control. There are hundreds of examples where companies contributed back to non-GPL projects because they are happily using it internally (closed) but still value the open source character of it. The GPL still has the mindset of evil corporations of the 90's. There are still evil players today, but everbody has to agree that the open source contributions of numerous companies cannot be ignored. The level of control the GPL forces on you, even if you want to write open source software, is insane and ridiculous. If you look at it closer, the GPL has the characteristics of cancer or a parasite. Additionally, by publishing your software under the GPL, most companies would not use your code and actually write their own version (which is most likely worse and full of bugs). This leads to the situation of many people actually having a really bad time with software, because the software they buy is actually full of horrible horrible code that could be avoided. In theory, the wonderland the GPL proposes "works". In reality, nowadays, it doesn't make a lot of sense any more. Richard Stallman used to be right. The companies of the 90's were not yet accustomed to an Open Source environment, but nowadays, his radical claims are just borderline insane. He for instance calls OpenBSD a non-free distribution, because they link to non-free software in the ports tree. Keep in mind that OpenBSD is completely blob-free, which is only achieved in the Linux-camp by obscure distributions nobody uses. So, the net-gain is this: The super-radical position of the FSF actually does more damage than it brings good, as people will never use the obscure FSF- distributions. They won't listen to rms's ramblings and songs either, because he still has not understood the changes the market has undergone in the last decade. We generally have to ask ourselves the question if we really should ramp up on the FSF anti-propaganda. The FSF has the biggest funding of all Open Source non-profit organizations (afaik), but what do they achieve relative to their size? I sometimes regret imagining what the OpenBSD foundation would do with all that money. They actually write useful software and make a positive impact. The FSF's initiatives, especially in regard to Gender mainstreaming and other marxist ideologies is, to say it likely, a long reach to computer science. Just food for thought, please don't start a discussion here about this. I don't care abour your opinion that much anyway. Cheers FRIGN -- FRIGN <d...@frign.de>
[hackers] [dwm] [PATCH] Replace str[n]cpy with strlcpy
Hello fellow hackers,
I'll drop this little patch here so we finally make the switch to the
safe OpenBSD-functions for string copying.
Read the patch description for more info.
Cheers
FRIGN
--
FRIGN <d...@frign.de>
>From 849a7cbee0310beb7ea51986bf98aff8d3b7ff26 Mon Sep 17 00:00:00 2001
From: FRIGN <d...@frign.de>
Date: Thu, 2 Jun 2016 21:46:50 +0200
Subject: [PATCH] Replace str[n]cpy with strlcpy
Let's finally use this safe interface here! We've waited long
enough.
Even if a call to strcpy in some cases might be safe
(e.g. writing the broken string to c->name), we can never
assure that there might not be a code change in the future
that breaks this assumption.
Even though you might have had these side-effects in mind the
time you wrote the code, they definitely won't be a few days/
months/years later when the changes are made.
---
dwm.c | 16 +---
util.c | 40
util.h | 3 +++
3 files changed, 52 insertions(+), 7 deletions(-)
diff --git a/dwm.c b/dwm.c
index ff7e096..d45916c 100644
--- a/dwm.c
+++ b/dwm.c
@@ -393,7 +393,7 @@ arrange(Monitor *m)
void
arrangemon(Monitor *m)
{
- strncpy(m->ltsymbol, m->lt[m->sellt]->symbol, sizeof m->ltsymbol);
+ strlcpy(m->ltsymbol, m->lt[m->sellt]->symbol, sizeof(m->ltsymbol));
if (m->lt[m->sellt]->arrange)
m->lt[m->sellt]->arrange(m);
}
@@ -654,7 +654,8 @@ createmon(void)
m->topbar = topbar;
m->lt[0] = [0];
m->lt[1] = [1 % LENGTH(layouts)];
- strncpy(m->ltsymbol, layouts[0].symbol, sizeof m->ltsymbol);
+ strlcpy(m->ltsymbol, layouts[0].symbol, sizeof(m->ltsymbol));
+
return m;
}
@@ -931,10 +932,10 @@ gettextprop(Window w, Atom atom, char *text, unsigned int size)
if (!name.nitems)
return 0;
if (name.encoding == XA_STRING)
- strncpy(text, (char *)name.value, size - 1);
+ strlcpy(text, (char *)name.value, size - 1);
else {
if (XmbTextPropertyToTextList(dpy, , , ) >= Success && n > 0 && *list) {
- strncpy(text, *list, size - 1);
+ strlcpy(text, *list, size - 1);
XFreeStringList(list);
}
}
@@ -1526,7 +1527,8 @@ setlayout(const Arg *arg)
selmon->sellt ^= 1;
if (arg && arg->v)
selmon->lt[selmon->sellt] = (Layout *)arg->v;
- strncpy(selmon->ltsymbol, selmon->lt[selmon->sellt]->symbol, sizeof selmon->ltsymbol);
+ strlcpy(selmon->ltsymbol, selmon->lt[selmon->sellt]->symbol,
+ sizeof(selmon->ltsymbol));
if (selmon->sel)
arrange(selmon);
else
@@ -1991,14 +1993,14 @@ updatetitle(Client *c)
if (!gettextprop(c->win, netatom[NetWMName], c->name, sizeof c->name))
gettextprop(c->win, XA_WM_NAME, c->name, sizeof c->name);
if (c->name[0] == '\0') /* hack to mark broken clients */
- strcpy(c->name, broken);
+ strlcpy(c->name, broken, sizeof(c->name));
}
void
updatestatus(void)
{
if (!gettextprop(root, XA_WM_NAME, stext, sizeof(stext)))
- strcpy(stext, "dwm-"VERSION);
+ strlcpy(stext, "dwm-"VERSION, sizeof(stext));
drawbar(selmon);
}
diff --git a/util.c b/util.c
index 6b703e9..ac4372f 100644
--- a/util.c
+++ b/util.c
@@ -31,3 +31,43 @@ die(const char *fmt, ...) {
exit(1);
}
+
+/*
+ * Copyright (c) 1998, 2015 Todd C. Miller <todd.mil...@courtesan.com>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+size_t
+strlcpy(char *dst, const char *src, size_t dsize)
+{
+ const char *osrc = src;
+ size_t nleft = dsize;
+
+ /* Copy as many bytes as will fit. */
+ if (nleft != 0) {
+ while (--nleft != 0) {
+ if ((*dst++ = *src++) == '\0')
+break;
+ }
+ }
+
+ /* Not enough room in dst, add NUL and traverse rest of src. */
+ if (nleft == 0) {
+ if (dsize != 0)
+ *dst = '\0'; /* NUL-terminate dst */
+ while (*src++)
+ ;
+ }
+
+ return(src - osrc - 1); /* count does not include NUL */
+}
diff --git a/util.h b/util.h
index cded043..ce29b77 100644
--- a/util.h
+++ b/util.h
@@ -1,4 +1,5 @@
/* See LICENSE file for copyright and license details. */
+#include
#define MAX(A, B) ((A) > (B) ? (A) : (B))
#define MIN(A, B) ((A) < (B) ? (A) : (B))
@@ -6,3 +7,5 @@
void die(const char *errstr, ...);
void *ecalloc(size_t, size_t);
+#undef strlcpy
+size_t strlcpy(char *, const char *, size_t);
--
2.4.10
Re: [hackers] [libzahl] Switch to ISC license. || Mattias Andrée
On Thu, 2 Jun 2016 12:54:39 +0200 Mattias Andrée <maand...@kth.se> wrote: Hey Mattias, > I generally think licenses are easier to grok > if they are long. They omit less information. > But I have to agree that ISC is easier to > understand than MIT. For one thing, it does > not use any words that require a legal dictionary > and most of it can be understood by a 10-year > old that does not even have English as her > native language. It also does not use any > fuzzy words like “substantial”. ISC leaves all > cruft and fuzzyness to copyright law. yeah and that's the good point about it. It's clear what it implies without hiding it behind long paragraphs. > Now, if we could only get rid of the disclaimer, > but I suspect it is required in some jurisdictions. It's required in all jurisdictions. I challenge you to find one where this isn't needed, afaik there is none. Commercial law is pretty clear in this case. If something blows your computer up, you could theoretically sue. So yeah, let's keep it. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers][ubase][tput] tput(1)
On Sun, 22 May 2016 16:01:43 -0300 Lucas Gabriel Vuotto <l.vuott...@gmail.com> wrote: Hey Lucas, > attached you will find two patches, one implementing a basic library for > dealing > with terminfo capabilities and another implementing the tput command using > that > library. The library itself is part of a bigger library from me, damned [0], > for > TUI creation. > 9 files changed, 2179 insertions(+), 3 deletions(-) :O definitely too much man. If we look at sbase, it has the following SLOC-count: Totals grouped by language (dominant language first): ansic:18805 (97.57%) awk:255 (1.32%) sh: 213 (1.11%) do you really think it's a good idea to increase its size by roughly 10% to accomodate one single command? Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [farbfeld] [PATCH] Improve fread error handling in ff2*
On Thu, 19 May 2016 01:21:58 +0300 Alexander Krotov <ilab...@yandex.ru> wrote: Hey Alexander, > In case of unexpected end of file errno is not set, and strerror(errno) > returns "Success". Caller should distinguish between error and EOF by > calling ferror(3) as described in fread(3). thank you very much for this patch! I've applied it. Very good work! Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [dwm][PATCH] Corrected typo.
On Mon, 25 Apr 2016 09:48:44 +1200 David Phillips <dbphillip...@gmail.com> wrote: Hey David, > What typo is this fixing? The only change I can see is the capitalisation is > being changed against the grain of the rest of the codebase. > > Please corect me if I am wrong. You are right. It's general convention that we don't use capital letters here. That's why this patch is wrong. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [farbfeld] Remove dimension checks || FRIGN
On Sun, 10 Apr 2016 23:49:49 +0100 Dimitris Papastamos <s...@2f30.org> wrote: Hey Dimitris, > One problem I see with this approach is that an error will potentially go > unnoticed and require more time to diagnose than necessary. it's the same debate as with UTF-parsers and how liberal they should be with input data. >From my experience, being liberal with input sounds nice in theory, but really can lead to slacky and broken code in practice. If garbage is fed to the farbfeld tools, I really cannot stress this enough, we will try to work with the garbage as far as possible. And given farbfeld explicitly allows 0x0 pixel images (and 0x3 for that matter), we accept it as input. Are these dimensions useful? Nah, but they are legal and that's why I don't want to error out prematurely. I think it's better to let the image libraries handle this case. What we _could_ do is error out and say "libpng does not support zero dimension images" however, this would be a statement based on one current version of libpng, and we cannot predict how the format will develop. The png_set_iHDR-function will error out if we pass bad data to it, so why bother in the main code? Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [sbase][PATCH] grep: remove = flag from readme
On Sun, 3 Apr 2016 23:35:21 +0200 Mattias Andrée <maand...@kth.se> wrote: Hey Mattias, > Perhaps it could be noted. But it fails > with both musl and glibc. I think we need new > regex engine, perhaps adapted from musl, > that supports NUL bytes and UTF-8 transparently. you could also try a setlocale(LC_ALL, "UTF-8"); in grep(1). Maybe that works. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [sbase][PATCH] grep: add -r
On Wed, 30 Mar 2016 05:23:08 +0200 Mattias Andrée <maand...@kth.se> wrote: > Unlike your usual grep -r, this implementation > uses breadth-first search. It usually finds > makes it find what you are looking for faster. Try working with recurse() in libutil. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [sbase][PATCH] grep: remove = flag from readme
On Wed, 30 Mar 2016 19:01:16 +0200 Mattias Andrée <maand...@kth.se> wrote: Hey Mattias, > $ echo äö | ./grep [å] > äö > > This is not want one expects from > a program that supports UTF-8. I assume the proper way here would be to add a note on the manpage that this highly depends on the libc you are using (and esp. the Regex engine). Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [libzahl] Fix example in comments
On Mon, 7 Mar 2016 14:08:04 +0200 Vasily Kolobkov <polezaivs...@openmailbox.org> wrote: > - = 40⋅30⋅10⁴ + (40⋅20 + 30⋅10)⋅10² + 10⋅20, but the middle is > + = 4⋅3⋅10⁴ + (40⋅20 + 30⋅10)⋅10² + 10⋅20, but the middle is 4000 * 3000 = 40 * 30 * 10^4 = 4 * 3 * 10^6. -- FRIGN <d...@frign.de>
Re: [hackers] [farbfeld] Fix 2 little things in jpg2ff(1) || FRIGN
On Sun, 6 Mar 2016 07:47:38 +0100
k...@shike2.com wrote:
Hey Roberto,
> > - if (argc > 1) {
> > + if (argc) {
> > fprintf(stderr, "usage: %s\n", argv0);
> > return 1;
> > }
> recommendation about style; Use argc == 0 here and use the form
> without relational operator for logic or pointers values.
You know how it is with style, but I respect your opinion on this
matter as a senior programmer.
I was wondering, don't you mean "argc != 0"? :)
Cheers
FRIGN
--
FRIGN <d...@frign.de>
Re: [hackers] [libzahl] Add a number of man pages || Mattias Andrée
On Mon, 29 Feb 2016 15:34:42 +0100 (CET) g...@suckless.org wrote: > Add a number of man pages I swear to you, you are the only guy I know who writes the manuals before the code. -- FRIGN <d...@frign.de>
Re: [hackers] [sbase] Rever the strmem() addition and add a TODO element || FRIGN
On Fri, 26 Feb 2016 10:54:55 +0100 (CET) g...@suckless.org wrote: > The "real" solution is to use memmem() where needed and replace all > functions that assume zero-terminated-strings from standard input, which > could lead to early string-breakoffs. > This requires a strict tracking of string lengths. A short comment here: Of course I'll import memmem from OpenBSD into libutil, given it's a GNU extension. -- FRIGN <d...@frign.de>
Re: [hackers] dmenu][PATCH reversed
On Sat, 20 Feb 2016 22:28:14 +0100
Maurycy Skier <hahis...@gmail.com> wrote:
Hey Maurycy,
> Hi, I hope I am doing everything right.
sadly no. Please better send your patch using git format-patch.
You do it like this:
Make your changes on the files, then do git add, git commit and then
do "git format-patch -1" to generate a patch with proper description
and authorship.
> P.S. You could also add another target to Makefile: git-diff. It'd go
> like this:
> git-diff:
> git-diff > dmenu-${VERSION}-${PATCHNAME}.diff
Absolutely not, see above.
Thanks for your patch, but please resend it with git format-patch in a
response to this thread! :)
Cheers
FRIGN
--
FRIGN <d...@frign.de>
Re: [hackers] [slock] Use argv0 instead of passing "slock:" to die every time || FRIGN
On Mon, 15 Feb 2016 10:34:15 + Dimitris Papastamos <s...@2f30.org> wrote: Hey Dimitris, > Yes this pattern is used in sbase. There is no point however > replicating it to other projects. so how would you approach this? If slock was only a main()-function, one could've thought to just pass argv[0] to each call of die. But it is not. Would your approach be to just set argv0 as argv[0] and carry on without modifying argc, argv? I was surprised this idiomatic one-liner has received such opposition. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [slock] No need for usage() || FRIGN
On Mon, 15 Feb 2016 11:15:50 +0100 Markus Teich <markus.te...@stusta.mhn.de> wrote: Hey Markus, > I have to agree with Christoph here. People running off git can just use the > rev > id they are using. I don't get what you mean with the attack surface sentence. it was a rather weak argument by me, but still a possibility. Say somebody is running an older version of slock, an "attacker" could probe it out. However, after further consideration, this could also be done by examining the behaviour of the problem while locked to identify an older version. Nevermind then. > I think we can ignore the possibility of someone wanting to call his custom > `-h` > or `-v` binary when the screen is locked and revert the commit. My considerations here were that it was quite arbitrary not to document -h, given we "allow" a command to be passed to slock as second + further arguments. However, I respect your stances on this and will revert it, but also document -h in the manpage. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [slock] Use argv0 instead of passing "slock:" to die every time || FRIGN
On Mon, 15 Feb 2016 11:22:08 +0100 Markus Teich <markus.te...@stusta.mhn.de> wrote: Hey Markus, > why this `argc--, argv++` shenanigans? I think it's more confusing rather than > helping. we came up with this in sbase/ubase as an idiomatic way to set argv0 for tools that don't use arg.h. We need argv0 here because I moved the prepending of the argv[0] into die(), and thus we need to store the argv0 in a global variable. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [ubase] mount: fix mount helper fs option handling || Brad Barden
On Thu, 11 Feb 2016 11:16:47 +0100 (CET) g...@suckless.org wrote: > + if (strlcat(fsopts, EARGF(usage()), sizeof(fsopts)) >= > sizeof(fsopts)) > + if (strlcat(fsopts, me->mnt_opts, > sizeof(fsopts)) >= sizeof(fsopts)) Use estrlcat() instead. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [farbfeld] Mandate "ProPhoto RGB" color space for farbfeld and handle ICC profiles || FRIGN
On Sun, 17 Jan 2016 10:10:19 + Dimitris Papastamos <s...@2f30.org> wrote: > Commit message reads like a diary. That's because it is a diary. :P -- FRIGN <d...@frign.de>
Re: [hackers] [PATCH] yellow italics everywhere is for colorblind people
On Tue, 5 Jan 2016 17:44:12 +0800 Pickfire <pickf...@riseup.net> wrote: > I doesn't know there is an ml etiquette, I will add it to the wiki. > It isn't mentioned. Not everything is written in the wiki man, and it's not the first time you sent in a patch. You know how patch-mails are formatted, so that's how you can deduce a ml-etiquette easily. But feel free to send in a patch, so the newcomers know what it's about. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [PATCH] yellow italics everywhere is for colorblind people
On Tue, 5 Jan 2016 17:38:16 +0800 Pickfire <pickf...@riseup.net> wrote: > Hi, I use `git send-email`, it won't be mentioned by default. > It is for st. As you can see in the patch. I don't care what you use. Just stay with the ml etiquette and mention the project name in the title of your bloody mail. -- FRIGN <d...@frign.de>
Re: [hackers] [tabbed] Bump year. || Christoph Lohmann
On Fri, 1 Jan 2016 14:19:29 +0100 (CET)
g...@suckless.org wrote:
> - die("tabbed-"VERSION", © 2009-2014"
> + die("tabbed-"VERSION", © 2009-2015"
You probably mean 2016.
--
FRIGN <d...@frign.de>
Re: [hackers] [sbase] chown: fix user:group option parsing
On Mon, 21 Dec 2015 19:05:17 + Dimitris Papastamos <s...@2f30.org> wrote: > uid_t, gid_t are typically unsigned, so the check needs to be > == -1 otherwise it will never be true. We both are wrong :P It can either be signed or unsigned. -- FRIGN <d...@frign.de>
Re: [hackers] [sbase] chown: fix user:group option parsing
On Mon, 21 Dec 2015 19:37:10 +0100 Quentin Rameau <quinq@fifth.space> wrote: > + if (uid == -1 && gid == -1) > + usage(); better use < 0 than -1 -- FRIGN <d...@frign.de>
Re: [hackers] [sbase] Cleanup usage() across sbase
On Mon, 21 Dec 2015 17:20:34 +0100
Quentin Rameau <quinq@fifth.space> wrote:
Hey Quentin,
> Some tools didn't use argv0 for tool name, or usage() at all.
thanks for your patch, though there are some issues with it I'd like to point
out now.
> +#include "arg.h"
There's no need to include arg.h. This is already included in util.h, so let's
keep it
this way and not include it.
> diff --git a/chown.c b/chown.c
>
> - eprintf("usage: %s [-h] [-R [-H | -L | -P]] [owner][:[group]] file
> ...\n", argv0);
> + eprintf("usage: %s [-h] [-R [-H | -L | -P]] owner[:group] file ...\n"
> + " %s [-h] [-R [-H | -L | -P]] :group file ...\n",
> + argv0, argv0);
Don't change the usage as is, unless you also change the manpage meanfully.
I see what you did there, but if at all, make it a separate patch.
> diff --git a/cksum.c b/cksum.c
>
> +static void
> +usage(void)
> +{
> + eprintf("usage: %s [file ...]\n", argv0);
> +}
> +
> int
> main(int argc, char *argv[])
> {
> FILE *fp;
>
> - argv0 = argv[0], argc--, argv++;
> + ARGBEGIN {
> + default:
> + usage();
> + } ARGEND
NO! The rule of thumb is: If a tool doesn't accept any flags, we won't
handle flags and rather look for files called "-f" or something.
> diff --git a/ed.c b/ed.c
Ok.
> diff --git a/find.c b/find.c
Ok.
> diff --git a/grep.c b/grep.c
Ok.
> diff --git a/join.c b/join.c
>
> - eprintf("usage: %s [-1 field] [-2 field] [-o list] [-e string] "
> - "[-a | -v fileno] [-t delim] file1 file2\n", argv0);
> + eprintf("usage: %s [-1 field] [-2 field] [-a fileno | -v fileno] "
> + "[-e string] [-o list] [-t delim] file1 file2\n", argv0);
Look at the manpage. The non-alphabetical order has a reason. If at all,
the usage-strings can only be changed when the manpage is changes as well.
> diff --git a/nl.c b/nl.c
> eprintf("usage: %s [-p] [-b type] [-d delim] [-f type]\n"
> - " [-h type] [-i num] [-l num] [-n format]\n"
> - " [-s sep] [-v num] [-w num] [file]\n", argv0);
> + " %*s [-h type] [-i num] [-l num] [-n format]\n"
> + " %*s [-s sep] [-v num] [-w num] [file]\n",
> + argv0, strlen(argv0), " ", strlen(argv0), " ");
What the fuck? Just keep the 7 spaces, alright? We don't need to become
dynamic here.
> diff --git a/renice.c b/renice.c
Ok.
> diff --git a/seq.c b/seq.c
>
> - eprintf("usage: %s [-f fmt] [-s sep] [-w] [startnum"
> -" [step]] endnum\n", argv0);
> + eprintf("usage: %s [-f fmt] [-s sep] [-w] [startnum [step]] endnum\n",
> + argv0);
Why break the line limit here? Just keep it as-is.
> diff --git a/sort.c b/sort.c
>
> enprintf(2, "usage: %s [-Cbcmnru] [-o outfile] [-t delim] "
> - "[-k def]... [file ...]\n", argv0);
> + "[[-k def] ...] [file ...]\n", argv0);
Please make that a separate patch and change the manpage accordingly.
> diff --git a/tar.c b/tar.c
Ok.
> diff --git a/touch.c b/touch.c
Ok.
> diff --git a/tr.c b/tr.c
>
> static void
> usage(void)
> {
> - eprintf("usage: %s [-cCds] set1 [set2]\n", argv0);
> + eprintf("usage: %s [-Ccds] set1 [set2]\n", argv0);
Same here. Change the manpage.
> diff --git a/xargs.c b/xargs.c
Ok.
Cheers
FRIGN
--
FRIGN <d...@frign.de>
Re: [hackers] [sbase] Cleanup usage() across sbase
On Mon, 21 Dec 2015 18:36:28 +0100 Quentin Rameau <quinq@fifth.space> wrote: > Some tools didn't use argv0 for tool name, or usage() at all. Thanks, applied! :) http://git.2f30.org/sbase/commit/?id=fd70f2afbeb1cacc0d55aa47831c84f39ec80978 -- FRIGN <d...@frign.de>
Re: [hackers] [sent] [PATCH 2/2] replace farbfeld with libnetpbm
On Sun, 13 Dec 2015 11:22:30 +0100 s...@mailless.org wrote: > ironically, I don't have imagemagick installed on any of my boxes, but > have netpbm on all. (but then I can use any of the numerous ppmto* and > pnmto* and pamto* to convert to whatever I need.) I don't blame you, imagemagick is a beast. But if you do graphics stuff, the pdf- and other utilites are very handy. > that said, however, I don't really get why one would insist on having > pbm as intermediate format, apart from "because you can". Exactly. > for me, pbm is a medium which allows to apply all the tools available in > the netpbm suite on a bulk of images, and there are quite some of them. I'll think about adding pbm2ff and ff2pbm to the farbfeld-utilities :P Then the crowd is happy and you can use the netpbm suite. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [sent] [PATCH 2/2] replace farbfeld with libnetpbm
On Sun, 13 Dec 2015 11:26:34 +1300 dbphillip...@gmail.com wrote: > Valid observation, although this patch is about changing the intermediate > format, not the input format. I know, but what argument can you give? I mean, this entire thing is only based on spread- and team-arguments. The technical side has been lacking for quite a while now. I understand, netpbm is _the_ format here and I shouldn't dare to question the fact that it is widespread as hell. But apart from widespread, what is a reason to use it? The library interface is as cumbersome to use as libpng. There are good reasons why sent doesn't use PNG as an intermediate format. So, what is the reason? What does netpbm really _do_ better than farbfeld? And no, Roberto, writing an image in ed does not cut it. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [sent] [PATCH 2/2] replace farbfeld with libnetpbm
On Wed, 9 Dec 2015 20:56:38 -0800 Grant Mathews <grant.m.math...@gmail.com> wrote: Hey Grant, > Since the PNM/PAM format already exist as a minimal intermediate > representation with a rich set of commandline tools to manipulate them, > use Netpbm to handle images. back to your desk, Grant, you've missed the purpose of farbfeld. No, to be fair, we discussed the PAM-format at slcon2 in more than one coffee break. Netpbm is arguably almost more complex than BMP and not easy to handle. We could discuss it if it was widely used, but the main point is: __You need a library to handle it__ and it's not that much of a popular format to justify installing a library for it. I don't know about you guys, but I don't have libpbm installed on my computer and even though ffmpeg for instance offers support, I might be having a hard time finding a format ffmpeg _doesn't_ support. You've replaced the entire farbfeld parsing code in sent with the boilerplate "offered" by netpbm. Good job! A question for the diligent reader: Can you read in a netpbm file without first looking into the docs? Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [farbfeld] jpg2ff: fix segfault on JPG error
On Tue, 8 Dec 2015 11:28:17 +0100 Quentin Rameau <quinq+hackers@fifth.space> wrote: > We would try to free unitialized ff_row pointer in cleanup on jpeg > errors. thanks, applied! :) -- FRIGN <d...@frign.de>
Re: [hackers] [PATCH] [sent] Quick patch to replace png with farbfeld
On Wed, 18 Nov 2015 10:46:51 + Dimitris Papastamos <s...@2f30.org> wrote: > This also works with jpg and gif and others. The 2ff wrapper will > use imagemagick conversion tools. This is temporary as jpg2ff and > gif2ff will also be implemented. As a side note, the 2ff-filter can also receive png-images and already automatically call png2ff when it detects a png. If you look at the 2ff script, you'll notice how simple it is really. My plan is, to, over time, add the other jpg2ff, gif2ff and what-not and slowly replace the imagemagick-fallback. Not everybody has imagemagick on his computer, but can still run 2ff depending on what he passes to it. imagemagick may be a sucky program as is, but just as with surf to access the web, I see 2ff like a glove to touch all these diseases image- formats (like png, gif, jpg, tiff, ...) not having to worry about them while working with the actual data. I again want to stress here that I've been dealing with libpng for almost 2 days now and I've still not found a satisfying general way to handle 16-Bit PNG's properly. The interface can be compared to the 6th circle of hell. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [PATCH] [sent] Quick patch to replace png with farbfeld
On Tue, 17 Nov 2015 15:01:40 + Dimitris Papastamos <s...@2f30.org> wrote: Hey Dimitris, > Very quick and dirty patch. This is intended to generate a discussion > on the applicability of farbfeld in sent. I can polish the patch > later depending on the outcome of the discussion. I really like this patch. Farbfeld contains some ingenious ideas, and blessing sent with it was a change due to happen. Now, to dive into this idea a bit more, farbfeld is a great intermediate format. If you store farbfeld without compression on your hard drive, you kind of missed the point of it. Farbfeld is very unixy, as it benefits from pipelined-solutions. sent is not a pipe-based system, and to be honest, I've not yet come up with a good way to either decompress farbfeld-compressed images or ad-hoc running png's through png2ff and supplying the stream to the program. Any ideas in this direction are much appreciated. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [PATCH] [sent] Quick patch to replace png with farbfeld
On Tue, 17 Nov 2015 18:07:01 +0100 ACE <a.mad.co...@gmail.com> wrote: Hey ACE, > Imagine publishing the slides with farbfeld images to the public. if you had read the responses more carefully, you would have seen that the plan is not to store plain farbfeld files in the talk-folder, but rather use farbfeld only as an internal format to make the code simpler and use the farbfeld-tools for conversion. > I don't see farbfeld as a viable format to publish data today. I'm not > hating on the farbfeld format, it just doesn't seem like it will get > the widespread adoption to be a viable format to use when publishing > data. The spec is not even a week old. What do you expect? :P > I would compare farbfeld with *.xdoc format; my machines currently have > no way of opening them. My current process of opening *.xdoc documents > involves borrowing a pc with MS word on it and convert it to PDF. > Why insist on obfuscating the data? (as in lack of support of using the > farbfeld format) actually, we are not obfuscating the data. the problem is that libpng is almost impossible to use properly in a C-program. With farbfeld, we would only have to worry about it in png2ff and ff2png, especially in regard to handling 64-Bit PNGs and other gems. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] [dmenu] paste fix
On Sun, 4 Oct 2015 18:42:23 +0200 Hiltjo Posthuma <hil...@codemadness.org> wrote: > It is correct to use if it's a (feature) patch for the wiki. For > patches against the current code it is encouraged to use format-patch > so they can be applied directly (with git am). The problem I see is that most of the time, people throw their patches on the wiki and never care about it. Especially for the git-patches, you have to update them every few months so they don't break against git-head. Given there's no easy way to preserve the "commit-message" at the top of the git-formatted patch, there is quite an inhibition to update them. Those patches applied against specific versions often are applied against tarballs, which don't employ git. So, in both cases, git format-patch sounds good in theory, but fails. It only makes sense for user-supplied patches expected to be applied to mainline, where you want to make sure that historic accuracy with specific commit-hashes is guaranteed. In any other case, it doesn't bring much to the table. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] od: added support for different size values
On Thu, 1 Oct 2015 14:10:39 -0400 Greg Reagle <greg.rea...@umbc.edu> wrote: Hey Greg, > See attached. I honestly do not like this patch. This will make it very difficult to add variable integer lengths later on. What I can think of is approaching this issue by emalloc'ing (BUFSIZ * size) bytes and use the fact that read() allows us to specify the length of the chunks. If we have 16 Bytes per line (bpl), we just read in (bpl / size) chunks. This allows us to solve line-break issues in the middle of a chunk and could even allow us to keep the static BUFSIZ-buffer (read won't read in partial chunks, problem solved). Then we can pass a void * of the read chunk to print value(). Inside print value, we know the size of the memory segment, so what we do is assemble a long long value from the memory (one simple for-loop). The advantage of this is that we don't have to butch the main loop too much and we open the way for a more general approach. This would also allow us to print 3-byte-chunks effortlessly, which the GNU coreutils cannot. If I hadn't an exam coming up next week, I'd do it myself. If you have no idea what the hell I'm talking about, wait until next Wednesday, when I've taken the exam. On thing left to discuss would be how to handle the printing. I'd heuristically set the format on the next major integral type. For instance, when we read in a 3 byte chunk, we want to take the next larger integral type (which is int32 with 4 bytes). Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] sbase: od: properly concatenate multiple file arguments
On Fri, 2 Oct 2015 10:40:12 -0400 Greg Reagle <greg.rea...@umbc.edu> wrote: > Relative to master. Better rename it to "lastfile". Then it's clearer ;) Also: - off_t addr; + static off_t addr = 0; please just keep the initialization inside the for-loop. It's a slightly better style than setting the variable directly at the declaration block. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] sbase: od: properly concatenate multiple file arguments
On Fri, 2 Oct 2015 11:16:24 -0400 Greg Reagle <greg.rea...@umbc.edu> wrote: > But I don't want it initialized every time the for loop is entered. > That's why I made it static. It needs to preserve its value between calls. Ah yeah, of course. Sorry, I overlooked that. Let's focus one one single thing first, or you end up rewriting all your patches anyway. We first need to get the length-handling properly first. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] od: added options -b and -d; bug fix affecting size C
On Fri, 2 Oct 2015 03:07:00 -0400 Greg Reagle <greg.rea...@umbc.edu> wrote: Hey Greg, > Trying again. keep in mind that these flags are XSI-extensions, which we generally don't include in sbase. Let's discuss this on IRC, given the code-changes are not that drastic. Cheers FRIGN -- FRIGN <d...@frign.de>
Re: [hackers] od: added options -b and -d; bug fix affecting size C
On Fri, 2 Oct 2015 03:28:34 -0400 Greg Reagle <greg.rea...@umbc.edu> wrote: Hey Greg, > I am not inclined to join in the IRC because of the large time difference. > It > is 3:30 AM here and I am awake only because I have insomnia and I'll be back > in > bed soon. How about discussing it in d...@suckless.org? Or you can discuss > it > without me on IRC, in which case let me know the result of your discussion. that's fine! :) > If you'll forgive my ignorance, I am learning about od by reading the man > page > from section 1posix. How do I learn about which features are XSI extensions? > Thanks. Use the OpenGroup-specs. od(1) for instance can be found here[0]. Make sure it specifies "IEEE Std 1003.1, 2013 Edition" at the top, so you know it's the 2013 corrigendum of POSIX 2013. XSI-extensions are then easily to see, being marked with "[XSI]". Now concerning the flags, -b and -c are on the one hand easy to implement. On the other hand though, this just shows how superfluous these are. Why do we have "-b" when it's equivalent to "-t o1"? Why do we have "-d" when it's equivalent to "-t u2"? And what about the other XSI-extensions? Why don't we include them? "-c" requires you to handle stuff with LC_CTYPE, which is a huge brainfuck as well. We don't handle XSI because we're too lazy, but because every time I read a POSIX spec and see [XSI], I read [BLOAT]. If you go on implementing XSI-extensions, all other [XSI]-extensions on the page become mandatory for the program to be POSIX-compliant. This includes also implementing the -o, -s, -x flags and sections like: "Multiple types can be specified by using multiple -bcdostx options. Output lines are written for each type specified in the order in which the types are specified." "[+]offset[.][b] The offset operand specifies the offset in the file where dumping is to commence. This operand is normally interpreted as octal bytes. If '.' is appended, the offset shall be interpreted in decimal. If 'b' is appended, the offset shall be interpreted in units of 512 bytes." "NLSPATH Determine the location of message catalogs for the processing of LC_MESSAGES." Due to the fucked up syntax of the offset-operand, beware to also check out APPLICATION USAGE in the informative section: "XSI-conformant applications are warned not to use filenames starting with '+' or a first operand starting with a numeric character so that the old functionality can be maintained by implementations, unless they specify one of the -A, -j, or -N options. To guarantee that one of these filenames is always interpreted as a filename, an application could always specify the address base format with the -A option." Do you really want that? Let's not touch the forbidden fruit. Cheers FRIGN [0]: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/od.html -- FRIGN <d...@frign.de>
Re: [hackers] created od
On Mon, 28 Sep 2015 21:06:58 -0400
Greg Reagle <greg.rea...@umbc.edu> wrote:
Hey Greg,
> This is very limited at this point.
thank you very much for your contribution! I haven't had the chance
to run the code yet, but let me first give you some bits on the code
itself.
I'm impressed how well you already achieved the sbase-style. Very
good work, especially regarding the argv-loop-idiom and other things!
1) © 2014,2015 Greg Reagle <greg.rea...@umbc.edu>
better use
© 2014-2015 Greg Reagle <greg.rea...@umbc.edu>
2) writes an octal dump of
.Ar file(s)
to standard output. If no
.Ar file(s)
are specified then
.Nm
is a filter, i.e. reads from standard input.
Better use consistent wording with the other manpages:
writes an octal dump of each
.Ar file
to stdout. If no
.Ar file
is given,
.Nm
reads from stdin.
The intent is good to point out that it acts as a filter then,
but if we add it here, it somehow would effectively be lacking
everywhere else.
The core utilities consist of many filters.
3) Try to avoid blank lines in mandoc. Debug more issues with
mandoc -Tlint.
4) #include
#include "stdlib.h"
#include "util.h"
There is an obvious error here. ;)
5) As I've not yet run the code, I'm a bit confused by the
deployment of usage() here. Is it possible usage is
printed after the program has given output?
If yes, rather try to work with exit codes and handle
those errors more or less centrally in main().
As soon as we have printed something to the opened files,
it's desirable to run fshut() on the given files as well
to flush the buffers and occasionally properly report
conditions when we run out of space (pipe to /dev/full
for testing).
6) size_t n; /*actual size of the buffer*/
size_t i; /*tracks index within the buffer*/
Try adding spaces after inside the comments (/* ... */).
Also, why not think of a better naming to convey the
meaning of these variables better inside the code?
If that is not possible, then the comments are fine.
7) Try ordering variable declarations by size, starting with
structs, then names types (e.g., Rune), and then by size,
starting with largest and going to smallest (size_t, uint,
int, char, ...).
Unsigned types could be considered larger, as they can
represent absolutely larger numbers.
8) while ((n = fread(buf, 1, 5, fp_in))) {
Just use for(; n = fread(buf, 1, 5, fp_in; )
9) if ((strlen(address_radix) > 1) || (!strchr("doxn", address_radix[0])))
Break up this long statement. Break it after the OR.
10) for (i=0; i<n; ++i, ++addr) {
Don't forget the spacing ;)
for (i = 0; i < n; ++i, ++addr) {
11) Feel free to also remove od from the TODO in your patch
and add it to the README (of course, without audit, but
in the process of POSIX compliance[0])
Again, thank you very much for your work! This is a pretty
important tool. :)
I'll also take my time and comment on your patch adding the
t-flag.
Cheers
FRIGN
[0]: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/od.html
--
FRIGN <d...@frign.de>
Re: [hackers] [sbase] Audit sort(1) and mark it as finished || FRIGN
On Tue, 4 Aug 2015 13:09:00 +0200 (CEST) g...@suckless.org wrote: 6) OFF-POSIX: Posix for no apparent reason does not allow more than one file when the -c or -C flags are given. This can be problematic when you want to check multiple files. With the change 5), rewriting check() to return a value, I went off-posix after discussing this with Dimitris to just allow arbitrary numbers of files. Obviously, this does not break scripts and is convenient for everybody who wants to quickly check a big amount of files. As soon as 1 file is unsorted, the return value is 1, as expected. For convenience reasons, check()'s warning now includes the filename. As a quick note, thanks to Dimitris we found out that Solaris also does it like that. We are not alone in this world. -- FRIGN d...@frign.de
[hackers] [st] [5 PATCHES] Code layout changes
Good evening everybody!
I've taken a look at st today and made some changes that have proven to
improve the code while working on sbase and ubase in the last few months.
These patches imply layout and style changes in most places of the code,
so before you grab your pitchforks and torches:
The patches on the wiki are broken for git HEAD anyway. The next thing on
my list is to go through _all_ patches on the wiki and make releases for
0.6, which has been tagged yesterday, and git HEAD preferrably after these
patches have been merged.
I know about the clause in the suckless style guide: The project's style
has precedence. Nevertheless, I think it's only beneficial to have a
consistent style across the multiple projects we have and most importantly
across a single project in particular.
I once read that suckless is indeed popular for some people because of our
great brain dead C dialect, thus I think it's a step in the right direction
to make this dialect even more solid.
Now is the best time to merge style changes like this, one day after the latest
version has been tagged. This gives us (and especially me) enough time to take
care of the wiki-patches.
Please let me know what you think about it!
Cheers
FRIGN
--
FRIGN d...@frign.de
From 584b615305589aa760d49e62cb7cf64d406d6653 Mon Sep 17 00:00:00 2001
From: FRIGN d...@frign.de
Date: Wed, 8 Jul 2015 17:00:16 +0200
Subject: [PATCH 1/5] Remove insane *_FILENO and EXIT_* usage
Any system having different assignments than the usual 0, 1, 2 for
the standard file numbers and 0, 1 for the exit-statuses is broken
beyond repair.
Let's keep it simple and just use the numbers, no reason to fall
out of the window here and bend down for POSIX.
---
st.c | 27 ---
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/st.c b/st.c
index b89d094..0260562 100644
--- a/st.c
+++ b/st.c
@@ -513,7 +513,7 @@ static STREscape strescseq;
static int cmdfd;
static pid_t pid;
static Selection sel;
-static int iofd = STDOUT_FILENO;
+static int iofd = 1;
static char **opt_cmd = NULL;
static char *opt_io = NULL;
static char *opt_title = NULL;
@@ -1207,7 +1207,7 @@ die(const char *errstr, ...) {
va_start(ap, errstr);
vfprintf(stderr, errstr, ap);
va_end(ap);
- exit(EXIT_FAILURE);
+ exit(1);
}
void
@@ -1256,12 +1256,12 @@ execsh(void) {
signal(SIGALRM, SIG_DFL);
execvp(prog, args);
- _exit(EXIT_FAILURE);
+ _exit(1);
}
void
sigchld(int a) {
- int stat, ret;
+ int stat;
pid_t p;
if((p = waitpid(pid, stat, WNOHANG)) 0)
@@ -1270,10 +1270,9 @@ sigchld(int a) {
if(pid != p)
return;
- ret = WIFEXITED(stat) ? WEXITSTATUS(stat) : EXIT_FAILURE;
- if (ret != EXIT_SUCCESS)
+ if (!WIFEXITED(stat) || WEXITSTATUS(stat))
die(child finished with error '%d'\n, stat);
- exit(EXIT_SUCCESS);
+ exit(0);
}
@@ -1308,9 +1307,7 @@ ttynew(void) {
if(opt_io) {
term.mode |= MODE_PRINT;
- iofd = (!strcmp(opt_io, -)) ?
- STDOUT_FILENO :
- open(opt_io, O_WRONLY | O_CREAT, 0666);
+ iofd = (!strcmp(opt_io, -)) ? 1 : open(opt_io, O_WRONLY | O_CREAT, 0666);
if(iofd 0) {
fprintf(stderr, Error opening %s:%s\n,
opt_io, strerror(errno));
@@ -1320,7 +1317,7 @@ ttynew(void) {
if (opt_line) {
if((cmdfd = open(opt_line, O_RDWR)) 0)
die(open line failed: %s\n, strerror(errno));
- close(STDIN_FILENO);
+ close(0);
dup(cmdfd);
stty();
return;
@@ -1337,9 +1334,9 @@ ttynew(void) {
case 0:
close(iofd);
setsid(); /* create a new process group */
- dup2(s, STDIN_FILENO);
- dup2(s, STDOUT_FILENO);
- dup2(s, STDERR_FILENO);
+ dup2(s, 0);
+ dup2(s, 1);
+ dup2(s, 2);
if(ioctl(s, TIOCSCTTY, NULL) 0)
die(ioctl TIOCSCTTY failed: %s\n, strerror(errno));
close(s);
@@ -3871,7 +3868,7 @@ cmessage(XEvent *e) {
} else if(e-xclient.data.l[0] == xw.wmdeletewin) {
/* Send SIGHUP to shell */
kill(pid, SIGHUP);
- exit(EXIT_SUCCESS);
+ exit(0);
}
}
--
1.8.5.5
From 2d60df69a6795509020216bff5b97734d80498cb Mon Sep 17 00:00:00 2001
From: FRIGN d...@frign.de
Date: Wed, 8 Jul 2015 17:18:46 +0200
Subject: [PATCH 2/5] Unboolify st
This practice proved itself in sbase, ubase and a couple of other
projects.
---
config.def.h | 3 +--
st.c | 45 ++---
2 files changed, 23 insertions(+), 25 deletions(-)
diff --git a/config.def.h b/config.def.h
index 64e75b8..5375c1c 100644
--- a/config.def.h
+++ b/config.def.h
@@ -30,7 +30,7 @@ static unsigned int doubleclicktimeout = 300;
static unsigned int tripleclicktimeout = 600;
/* alt screens */
-static bool allowaltscreen = true;
+static int allowaltscreen = 1;
/* frames per second st should at maximum draw to the screen */
static unsigned int xfps = 120;
@@ -381,4 +381,3 @@ static Key key[] = {
static uint selmasks[] = {
[SEL_RECTANGULAR] = Mod1Mask,
};
-
diff --git a/st.c b/st.c
index 0260562..7423840 100644
--- a/st.c
+++ b/st.c
@@ -6,7 +6,6 @@
#include locale.h
#include pwd.h
Re: [hackers] FILE vs fd
On Sat, 13 Jun 2015 10:06:04 +0530 Aditya Goturu aditya3...@gmail.com wrote: Is there any particular reason why I would use unix's fd and open() instead of ANSI's FILE struct and fopen()? Yes. -- FRIGN d...@frign.de
Re: [hackers] [sbase] [PATCH 1/2] join: get rid of strlen--fwrite barbarity
On Sat, 6 Jun 2015 13:07:26 -0400 Wolfgang Corcoran-Mathe first.lord.of.t...@gmail.com wrote: Hey Wolfgang, This gets rid of a barbarous strlen()/fwrite() construct. I have to admit, that it's not easy to decide which one is better. On the one hand, I favor good data structures and having the length at hand is a good thing. On the other hand, join is completed feature-wise, so why not just store the length on the heap locally in the function and be done with it? It's not barbarous at all. Cheers FRIGN -- FRIGN d...@frign.de
