Re: [hackers] [st][PATCH 00/24] odg patches - fix warnings/errors, plug leaks, tidying up

[David Phillips]

On Wed, Jan 23, 2019 at 03:54:35AM +0100, kais euchi wrote:
> Greetings !
> What's the point of all this if you are treating others who try to 
> participate this way ?
> I have never seen such lack of tact.
> This sucks.

Hi,

You must be new to the list.

Best regards,
David

Re: [hackers] [st][PATCH 03/24] Reduce variable scope where possible, fix cppcheck style warnings

[David Phillips]

Reducing variale scope goes against the "Do not mix declarations and code"
guideline [1]. While it's possible to reduce scope in some cases, it's
not always good form.

All subjective though.

[1] https://suckless.org/coding_style/

Re: [hackers] [st][PATCH 24/24] link with librt on non-openbsd systems, I forgot to uncomment

[David Phillips]

On Mon, Jan 21, 2019 at 11:56:39PM +, Oliver Galvin wrote:
> ---
>  config.mk | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/config.mk b/config.mk
> index 7c0018a..3de245b 100644
> --- a/config.mk
> +++ b/config.mk
> @@ -27,8 +27,8 @@ STCPPFLAGS = -DVERSION=\"$(VERSION)\" -D_XOPEN_SOURCE=600
>  UNAME = $(shell uname -a)
>  ifeq ($(UNAME),OpenBSD)
>   CPPFLAGS += -D_BSD_SOURCE
> -#else
> -#LIBS += -lrt
> +else
> + LIBS += -lrt
>  endif
>  
>  STCFLAGS = $(INCS) $(STCPPFLAGS) $(CPPFLAGS) $(CFLAGS) $(WARN)
> -- 
> 2.20.1
> 

You should have squashed/rebased this patch.

Re: [hackers] [PATCH] [sbase] Include sysmacros.h directly rather than types.h

[David Phillips]

On Mon, Oct 01, 2018 at 06:35:13PM -0700, Michael Forney wrote:
> On 2018-10-01, David Phillips  wrote:
> > Hi,
> >
> > Bumping this patch since sbase master now fails to build against
> > glibc 2.28
> >
> > Let me know if there are any improvements that could be made.
> >
> > Thanks,
> > David
> 
> I wish the #ifdef wasn't necessary, but it seems like the only way to
> use major/minor on glibc and non-glibc systems.
> 
> As far as I can tell, glibc has had sys/sysmacros.h for a long time,
> so it should be okay to always include it when __GLIBC__ is defined.
> 
> Rather than the if-else, I think we should always include sys/types.h,
> and also sys/sysmacros.h on glibc. I also think the comment is
> unnecessary. Do you mind if I apply with those two changes?

I agree with that logic, so long as that doesn't hurt BSD systems (I
can't recall at the moment if sys/types.h exists on BSD)

David

Re: [hackers] [PATCH] [sbase] ls: allow listing contents of directories with +r-x

[David Phillips]

On Tue, Sep 25, 2018 at 06:48:08PM -0700, Michael Forney wrote:
> 
> Rather than keeping track of the full prefix for `mkent` and `output`,
> what do you think about just tracking the directory fd, and using the
> *at family of functions to work relative to that directory? I attached
> a patch to show what I'm thinking. It looks like this might even
> simplify some of the code.
> 

I just read your patch in full and it looks good to me. It seems to
describe and exhibit the desired behaviour. One minor thing I would
propose adding is changing the stat error message to display the full
path rather than just the name of the file/directory that cannot be
stat-ed. This clarifies the source of errors in the case of `ls -R`-
style commands.

Regarding the stat failure, please find a patch attached based on your
patch which allows stat failure.

Thanks,
David
>From 856b408a098415d98e6c673c7d1df7a9dde0 Mon Sep 17 00:00:00 2001
From: David Phillips 
Date: Tue, 2 Oct 2018 15:11:36 +1300
Subject: [PATCH] ls: allow fstatat failure, don't output filenames on fstatat
 failure

This patch allows fstatat failure without stopping the ls operation
entirely. Instead, a validity flag is added to `struct entry` so that
output can list unknown file information as such.

A return code is added to mkent so that callers may avoid outputting a
filename if fstatat on that file fails. This avoids erroneously outputting
a filename when we don't know if it actually exists.
---
 ls.c | 40 +++-
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/ls.c b/ls.c
index 28d797a..a629503 100644
--- a/ls.c
+++ b/ls.c
@@ -33,6 +33,7 @@ struct entry {
dev_t   dev;
dev_t   rdev;
ino_t   ino, tino;
+   int valid;
 };
 
 static struct {
@@ -65,16 +66,22 @@ static int showdirs;
 
 static void ls(int, const char *, const struct entry *, int);
 
-static void
+static int
 mkent(int dirfd, struct entry *ent, char *path, int dostat, int follow)
 {
struct stat st;
 
ent->name = path;
if (!dostat)
-   return;
-   if (fstatat(dirfd, path, &st, follow ? 0 : AT_SYMLINK_NOFOLLOW) < 0)
-   eprintf("stat %s:", path);
+   return 0;
+   if (fstatat(dirfd, path, &st, follow ? 0 : AT_SYMLINK_NOFOLLOW) < 0) {
+   ent->valid = 0;
+   ret = 1;
+   weprintf("stat %s:", path);
+   return 1;
+   } else {
+   ent->valid = 1;
+   }
ent->mode  = st.st_mode;
ent->nlink = st.st_nlink;
ent->uid   = st.st_uid;
@@ -98,6 +105,7 @@ mkent(int dirfd, struct entry *ent, char *path, int dostat, 
int follow)
ent->tmode = ent->tino = 0;
}
}
+   return 0;
 }
 
 static char *
@@ -171,6 +179,13 @@ output(int dirfd, const struct entry *ent)
else
mode[0] = '?';
 
+   if (!ent->valid) {
+   printf("%c? ? ? ? ? ? ", mode[0]);
+   printname(ent->name);
+   puts(indicator(ent->mode));
+   return;
+   }
+
if (ent->mode & S_IRUSR) mode[1] = 'r';
if (ent->mode & S_IWUSR) mode[2] = 'w';
if (ent->mode & S_IXUSR) mode[3] = 'x';
@@ -271,8 +286,12 @@ lsdir(int dirfd, const char *path, const struct entry *dir)
continue;
 
ents = ereallocarray(ents, ++n, sizeof(*ents));
+   ent = &ents[n - 1];
mkent(dirfd, &ents[n - 1], estrdup(d->d_name), Fflag || iflag ||
lflag || pflag || Rflag || sort, Lflag);
+   if (!ent->valid) {
+   ent->mode = DTTOIF(d->d_type);
+   }
}
 
if (!Uflag)
@@ -445,15 +464,18 @@ main(int argc, char *argv[])
case 0: /* fallthrough */
*--argv = ".", ++argc;
case 1:
-   mkent(AT_FDCWD, &ent, argv[0], 1, Hflag || Lflag);
-   ls(AT_FDCWD, "", &ent, (!dflag && S_ISDIR(ent.mode)) ||
-   (S_ISLNK(ent.mode) && S_ISDIR(ent.tmode) &&
-!(dflag || Fflag || lflag)));
+   if (!mkent(AT_FDCWD, &ent, argv[0], 1, Hflag || Lflag)) {
+   ls(AT_FDCWD, "", &ent, (!dflag && S_ISDIR(ent.mode)) ||
+   (S_ISLNK(ent.mode) && S_ISDIR(ent.tmode) &&
+!(dflag || Fflag || lflag)));
+   }
 
break;
default:
for (i = ds = fs = 0, fents = dents = NULL; i < argc; ++i) {
-   mkent(AT_FDCWD, &ent, argv[i], 1, Hflag || Lflag);
+   if (mkent(AT_FDCWD, &ent, argv[i], 1, Hflag || Lflag)) {
+   continue;
+   }
 
if ((!dflag && S_ISDIR(ent.mode)) ||
(S_ISLNK(ent.mode) && S_ISDIR(ent.tmode) &&
-- 
2.18.0

Re: [hackers] [PATCH] [sbase] Include sysmacros.h directly rather than types.h

[David Phillips]

On Sun, Jul 08, 2018 at 10:51:44PM +1200, David Phillips wrote:
> 
> Reworked patch encompassing this behaviour is attached. Would
> be interested to see builds against glibc pre-2.25.
> 
> Let me know what you think.
> 
> Thanks,
> David

Hi,

Bumping this patch since sbase master now fails to build against
glibc 2.28

Let me know if there are any improvements that could be made.

Thanks,
David

Re: [hackers] [PATCH] [sbase] ls: allow listing contents of directories with +r-x

[David Phillips]

On Tue, Sep 25, 2018 at 06:48:08PM -0700, Michael Forney wrote:
> 
> Hi David,
> 

Hi there

> Rather than keeping track of the full prefix for `mkent` and `output`,
> what do you think about just tracking the directory fd, and using the
> *at family of functions to work relative to that directory? I attached
> a patch to show what I'm thinking. It looks like this might even
> simplify some of the code.

Looks better to me. I always felt that there was something wrong with
but wasn't aware of the solution you have now presented. Thanks for
looking at the patch. I will more thoroughly read and understand your
patch soon.

> Either way, I think the changes to avoid chdir and to show unknown
> entries (stat failure) should be separate commits.

Agreed. Not sure why they ended up in the same patch here aside from
human error.

Thanks,
David

Re: [hackers] [slock][patch] Prevent shift key from toggling to failed state

[David Phillips]

On Mon, Aug 27, 2018 at 02:16:38PM -0400, Michael Spradling wrote:
> Currently if the first character pressed is the shift key, slock moves
> to the failed state.  The failed state can modify the display and
> provide incorrect feedback to the user and also gives away that your
> first character required the shift key.
> ---
>  slock.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/slock.c b/slock.c
> index 5ae738c..e9dd1ad 100644
> --- a/slock.c
> +++ b/slock.c
> @@ -154,7 +154,9 @@ readpw(Display *dpy, struct xrandr *rr, struct lock 
> **locks, int nscreens,
>   IsKeypadKey(ksym) ||
>   IsMiscFunctionKey(ksym) ||
>   IsPFKey(ksym) ||
> - IsPrivateKeypadKey(ksym))
> + IsPrivateKeypadKey(ksym) ||
> + ksym == XK_Shift_L ||
> + ksym == XK_Shift_R)
>   continue;
>   switch (ksym) {
>   case XK_Return:
> -- 
> 2.18.0
> 

The patch Michael Buch linked should work for you. For background, I
created it because previous discussion hereabout 2 years ago showed
that the community rather keep slock extremely paranoid and ready to
trip to the failed state at any keypress in order to indicate any
form of tampering.

The control-clear patch tones that down a bit for those users who
don't need this fine a trigger.

Thanks,
David

Re: [hackers][sbase][PATCH] Fix build errors on some ARM64 Linux systems

[David Phillips]

On Mon, Aug 27, 2018 at 09:21:33PM +0200, Hiltjo Posthuma wrote:
> On Mon, Aug 27, 2018 at 09:37:24PM +0300, Platon Ryzhikov wrote:
> > These changes are required on Arch Linux ARM to build sbase. This does not 
> > affect x86 build.
> > ---
> >  ls.c  | 1 +
> >  tar.c | 1 +
> >  2 files changed, 2 insertions(+)
> > 
> > diff --git a/ls.c b/ls.c
> > index b716aba..9e79fcb 100644
> > --- a/ls.c
> > +++ b/ls.c
> > @@ -1,6 +1,7 @@
> >  /* See LICENSE file for copyright and license details. */
> >  #include 
> >  #include 
> > +#include 
> >  
> >  #include 
> >  #include 
> > diff --git a/tar.c b/tar.c
> > index a6ead2e..9359bfd 100644
> > --- a/tar.c
> > +++ b/tar.c
> > @@ -1,6 +1,7 @@
> >  /* See LICENSE file for copyright and license details. */
> >  #include 
> >  #include 
> > +#include 
> >  
> >  #include 
> >  #include 
> > -- 
> > 2.18.0
> > 
> 
> As reported before this is wrong and probably Linux and glibc-specific.
> 
> Also sort the include names.
> 
> -- 
> Kind regards,
> Hiltjo

How does my updated patch from Jul 8 on the previous report for this
bug look?


Thanks,
David

Re: [hackers] [PATCH] [sbase] ls: allow listing contents of directories with +r-x

[David Phillips]

As a side note to this patch: it's been sitting in my queue for
the better part of a year while I internally debated the
untidiness of not being allowed to chdir. It's an unfortunate
result of implementing this behaviour, but it's behaviour
that other implementations (namely GNU, but not Solaris) support.

Let me know any feedback or oversights on my part,
David

Re: [hackers] [PATCH] [sbase] Include sysmacros.h directly rather than types.h

[David Phillips]

On Mon, Jul 02, 2018 at 11:58:54PM +1200, David Phillips wrote:
> On Mon, Jul 02, 2018 at 01:20:42PM +0200, Quentin Rameau wrote:
> > Ok, the makedev(3) manpage from the man-pages states this indeed:
> > 
> > The BSDs expose the definitions for these macros via .
> > Depending on the version, glibc also exposes definitions for these
> > macros from that header file if suitable feature test macros are
> > defined. However, this behavior was deprecated in glibc 2.25, and since
> > glibc 2.28,  no longer provides these definitions.
> > 
> > musl still includes  from  so maybe we
> > would only need an #ifdef __GLIBC__ there…
> > 
> > Or a more complicated
> > #ifdef __GLIBC__
> > #if __GLIBC_PREREQ(2, 25)
> > #include 
> > #endif
> > #else
> > #include 
> > #endif
> 
> To simplify things on our side, couldn't we just include
>  if linking against glibc, regardless of its
> version?
> 
> It sounds like glibc has defined these in 
> for a while into the past (citation needed) and all that
> is going to change is the inclusion of this file from types.h
> 
> I know it's less "correct", but it doesn't seem prone to
> breakage if that assumption is correct.
> 
> Might not be worth the slight increase in clarity though, not
> really sure.
> 
> Thanks,
> David

Reworked patch encompassing this behaviour is attached. Would
be interested to see builds against glibc pre-2.25.

Let me know what you think.

Thanks,
David
>From 331a370ffd51876a98e81ef330f27631c4e46859 Mon Sep 17 00:00:00 2001
From: David Phillips 
Date: Mon, 2 Jul 2018 19:42:41 +1200
Subject: [PATCH] [sbase] On glibc, include sysmacros.h directly

On glibc, major, minor, and makedev are all defined in
sys/sysmacros.h with types.h only including this for historical
reasons. A future release of glibc will remove this behaviour,
meaning that major, minor, and makedev will no longer be defined
for us without including sysmacros.h.
---
 ls.c  | 7 +++
 tar.c | 8 
 2 files changed, 15 insertions(+)

diff --git a/ls.c b/ls.c
index b716aba..4c269bd 100644
--- a/ls.c
+++ b/ls.c
@@ -1,6 +1,13 @@
 /* See LICENSE file for copyright and license details. */
 #include 
+
+/* glibc 2.25+ deprecates bringing sysmacros.h in with types.h anymore
+ * to define major and minor */
+#ifdef __GLIBC__
+#include 
+#else
 #include 
+#endif
 
 #include 
 #include 
diff --git a/tar.c b/tar.c
index a6ead2e..c4a7435 100644
--- a/tar.c
+++ b/tar.c
@@ -2,6 +2,14 @@
 #include 
 #include 
 
+/* glibc 2.25+ deprecates bringing sysmacros.h in with types.h anymore
+ * to define major, minor, and makedev */
+#ifdef __GLIBC__
+#include 
+#else
+#include 
+#endif
+
 #include 
 #include 
 #include 
-- 
2.17.1

[hackers] [PATCH] [sbase] ls: allow listing contents of directories with +r-x

[David Phillips]

chdir()ing into a directory with +r-x fails, so we should manually use the
directory name as a prefix rather than chdir()ing into it.

Also adds new parameters to mkent for the prefix, and for dictating whether
or not a permission denied error when stat()ing shall be fatal or not. This
allows errors stat()ing children of `foo` to be reported and non-fatal, while
a permission denied error on `foo` itself would be fatal.

Also adds a valid flag to ent to hide stale/default stat so that unknown
permissions and user/groups can be displayed as such.
---
 ls.c | 89 +---
 1 file changed, 61 insertions(+), 28 deletions(-)

diff --git a/ls.c b/ls.c
index b716aba..076f6b7 100644
--- a/ls.c
+++ b/ls.c
@@ -10,11 +10,13 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "utf.h"
 #include "util.h"
 
 struct entry {
+   charvalid;
char   *name;
mode_t  mode, tmode;
nlink_t nlink;
@@ -58,15 +60,33 @@ static int showdirs;
 static void ls(const char *, const struct entry *, int);
 
 static void
-mkent(struct entry *ent, char *path, int dostat, int follow)
+mkent(struct entry *ent, char *prefix, char *name, int dostat, int follow,
+  int fatal)
 {
struct stat st;
+   char path[PATH_MAX];
 
-   ent->name = path;
+   ent->valid = 0;
+   ent->name = name;
if (!dostat)
return;
-   if ((follow ? stat : lstat)(path, &st) < 0)
-   eprintf("%s %s:", follow ? "stat" : "lstat", path);
+
+   if (prefix != NULL)
+   snprintf(path, PATH_MAX, "%s/%s", prefix, name);
+   else
+   strncpy(path, name, PATH_MAX);
+
+   if ((follow ? stat : lstat)(path, &st) < 0) {
+   if (errno == EACCES && !fatal) {
+   weprintf("%s %s:", follow ? "stat" : "lstat", path);
+   return;
+   } else {
+   eprintf("%s %s:", follow ? "stat" : "lstat", path);
+   }
+   } else {
+   ent->valid = 1;
+   }
+
ent->mode  = st.st_mode;
ent->nlink = st.st_nlink;
ent->uid   = st.st_uid;
@@ -130,12 +150,13 @@ printname(const char *name)
 }
 
 static void
-output(const struct entry *ent)
+output(const char *prefix, const struct entry *ent)
 {
struct group *gr;
struct passwd *pw;
struct tm *tm;
ssize_t len;
+   char path[PATH_MAX];
char *fmt, buf[BUFSIZ], pwname[_SC_LOGIN_NAME_MAX],
 grname[_SC_LOGIN_NAME_MAX], mode[] = "--";
 
@@ -163,6 +184,14 @@ output(const struct entry *ent)
else
mode[0] = '?';
 
+   if (!ent->valid) {
+   printf("%c? ? ? ? ? ? ", mode[0]);
+   printname(ent->name);
+   fputs(indicator(ent->mode), stdout);
+   putchar('\n');
+   return;
+   }
+
if (ent->mode & S_IRUSR) mode[1] = 'r';
if (ent->mode & S_IWUSR) mode[2] = 'w';
if (ent->mode & S_IXUSR) mode[3] = 'x';
@@ -208,7 +237,8 @@ output(const struct entry *ent)
printname(ent->name);
fputs(indicator(ent->mode), stdout);
if (S_ISLNK(ent->mode)) {
-   if ((len = readlink(ent->name, buf, sizeof(buf) - 1)) < 0)
+   snprintf(path, PATH_MAX, "%s/%s", prefix, ent->name);
+   if ((len = readlink(path, buf, sizeof(buf) - 1)) < 0)
eprintf("readlink %s:", ent->name);
buf[len] = '\0';
printf(" -> %s%s", buf, indicator(ent->tmode));
@@ -247,13 +277,15 @@ lsdir(const char *path, const struct entry *dir)
size_t i, n = 0;
char prefix[PATH_MAX];
 
-   if (!(dp = opendir(dir->name))) {
+   if (snprintf(prefix, PATH_MAX, "%s%s", path, dir->name) >=
+   PATH_MAX)
+   eprintf("path too long: %s%s\n", path, dir->name);
+
+   if (!(dp = opendir(prefix))) {
ret = 1;
-   weprintf("opendir %s%s:", path, dir->name);
+   weprintf("opendir %s:", prefix);
return;
}
-   if (chdir(dir->name) < 0)
-   eprintf("chdir %s:", dir->name);
 
while ((d = readdir(dp))) {
if (d->d_name[0] == '.' && !aflag && !Aflag)
@@ -264,8 +296,12 @@ lsdir(const char *path, const struct entry *dir)
continue;
 
ents = ereallocarray(ents, ++n, sizeof(*ents));
-   mkent(&ents[n - 1], estrdup(d->d_name), Fflag || iflag ||
-   lflag || pflag || Rflag || sort, Lflag);
+   ent = &ents[n - 1];
+   mkent(ent, prefix, estrdup(d->d_name), Fflag || iflag ||
+   lflag || pflag || Rflag || sort, Lflag, 0);
+   if (!ent->valid) {
+   ent->mode = DTTOIF(d->d_type);
+   }
}
 
closedir(dp);
@@ -279,13 +315,9

Re: [hackers] [PATCH] [sbase] Include sysmacros.h directly rather than types.h

[David Phillips]

On Mon, Jul 02, 2018 at 01:20:42PM +0200, Quentin Rameau wrote:
> Ok, the makedev(3) manpage from the man-pages states this indeed:
> 
> The BSDs expose the definitions for these macros via .
> Depending on the version, glibc also exposes definitions for these
> macros from that header file if suitable feature test macros are
> defined. However, this behavior was deprecated in glibc 2.25, and since
> glibc 2.28,  no longer provides these definitions.
> 
> musl still includes  from  so maybe we
> would only need an #ifdef __GLIBC__ there…
> 
> Or a more complicated
> #ifdef __GLIBC__
> #if __GLIBC_PREREQ(2, 25)
> #include 
> #endif
> #else
> #include 
> #endif

To simplify things on our side, couldn't we just include
 if linking against glibc, regardless of its
version?

It sounds like glibc has defined these in 
for a while into the past (citation needed) and all that
is going to change is the inclusion of this file from types.h

I know it's less "correct", but it doesn't seem prone to
breakage if that assumption is correct.

Might not be worth the slight increase in clarity though, not
really sure.

Thanks,
David

Re: [hackers] [PATCH] [sbase] Include sysmacros.h directly rather than types.h

[David Phillips]

On Mon, Jul 02, 2018 at 10:16:50AM +0200, Quentin Rameau wrote:
> > On glibc, major, minor, and makedev are all defined in
> > sys/sysmacros.h with types.h only including this for historical
> > reasons. A future release of glibc will remove this behaviour,
> > meaning that major, minor, and makedev will no longer be defined
> > for us without including sysmacros.h.
> 
> Could you link to the source of that information?
> 
> Thanks.

Sure thing. See the changelog for glibc-2.25 [1]. The manpage for
these functions aas provided by my current version of glibc (2.27)
says that the inclusion of sysmacros.h from types.h will be removed
by 2.28 (cannot find an alternative source for that at the moment).

Thanks,
David

[1]: https://sourceware.org/ml/libc-alpha/2017-02/msg00079.html

Re: [hackers] [PATCH] [sbase] Include sysmacros.h directly rather than types.h

[David Phillips]

On Mon, Jul 02, 2018 at 10:02:09AM +0200, Hiltjo Posthuma wrote:
>
> Did you test it with musl too and preferably other platforms?

Unfortunately not; I don't currently have access to non-glibc Linux
installations at the moment, nor BSDs. Perhaps users of those systems
might be able to chime in at some point.

Looking at some documentation, it looks like BSD will keep their
definitions in types.h, so this patch may need some rework with ifdefs.
Would be great to have some feedback on that one.


Thanks,
David

[hackers] [PATCH] [sbase] Include sysmacros.h directly rather than types.h

[David Phillips]

On glibc, major, minor, and makedev are all defined in
sys/sysmacros.h with types.h only including this for historical
reasons. A future release of glibc will remove this behaviour,
meaning that major, minor, and makedev will no longer be defined
for us without including sysmacros.h.
---
 ls.c  | 2 +-
 tar.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/ls.c b/ls.c
index b716aba..884776f 100644
--- a/ls.c
+++ b/ls.c
@@ -1,6 +1,6 @@
 /* See LICENSE file for copyright and license details. */
 #include 
-#include 
+#include 
 
 #include 
 #include 
diff --git a/tar.c b/tar.c
index a6ead2e..9359bfd 100644
--- a/tar.c
+++ b/tar.c
@@ -1,6 +1,7 @@
 /* See LICENSE file for copyright and license details. */
 #include 
 #include 
+#include 
 
 #include 
 #include 
-- 
2.17.1

Re: [hackers] [PATCH] Remove .hgtags

[David Phillips]

On Thu, May 10, 2018 at 01:05:12PM +0200, David Demelier wrote:
>  class="gmail_quote">Le 10 mai 2018 11:23, Hiltjo Posthuma 
>  a écrit : type="attribution">On Wed, May 
> 09, 2018 at 08:50:09PM +0200, David Demelier wrote:
> 
> > ---
> 
> >  .hgtags | 10 --
> 
> >  1 file changed, 10 deletions(-)
> 
> >  delete mode 100644 .hgtags
> 
> > 
> 
> > diff --git a/.hgtags b/.hgtags
> 
> > deleted file mode 100644
> 
> > index 98d1562..000
> 
> > --- a/.hgtags
> 
> > +++ /dev/null
> 
> > @@ -1,10 +0,0 @@
> 
> > -0391602b2f06e0c7e89b7328719bec5695f57d9c ii-1.2
> 
> > -daf8fc17d14014f312721c1d56ffa049392393fc ii-1.2
> 
> > -987fc9d57808948b4bcf626b096c9c60226455d3 ii-1.3
> 
> > -7c7c000b4f42a48676e8f98a953a981bf6b29029 1.4
> 
> > -4c6892284a9ae73de3c84c164e214d31e76427a4 1.5
> 
> > -4c6892284a9ae73de3c84c164e214d31e76427a4 1.5
> 
> > -6f504f412a5997158b651eac8785f8331408b2e5 1.5
> 
> > -6f504f412a5997158b651eac8785f8331408b2e5 1.5
> 
> > -550ee110071903b9676d848d8947e96e02a8a662 1.5
> 
> > -f09f802a80379d24194535c7991182ceeb9291af 1.6
> 
> > -- 
> 
> > 2.17.0
> 
> > 
> 
> > 
> 
> 
> 
> Please mention the project name in the subject message.
> 
> 
> 
> Patch looks OK.
>  dir="auto">Oops sorry, I'll do for the next ones. class="gmail_extra"> style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> dir="ltr">
> 
> 

Please don't send HTML email.

Thanks

Re: [hackers] [sent] [PATCH] Draw a progress bar on non-image slides

[David Phillips]

On Sun, Jan 07, 2018 at 12:23:50AM +0100, Markus Teich wrote:
> David Phillips wrote:
> > Similar to the slide numbers patch, it may be useful for an audience to know
> > how much more of a presentation they have to endure. A naïve approach to 
> > this
> > is to introduce a progress bar to the bottom of the slides which indicate 
> > the
> > presenter's progress based on slide count.
> 
> Heyho David,
> 
> Thanks for the patch! Please add it to the wiki: https://suckless.org/wiki
> 
> --Markus

Perfect, that's what I was expecting. Thought I would try floating it for
mainline anyway.

Cheers,
David

[hackers] [sent] [PATCH] Draw a progress bar on non-image slides

[David Phillips]

Similar to the slide numbers patch, it may be useful for an audience
to know how much more of a presentation they have to endure. A naïve
approach to this is to introduce a progress bar to the bottom of the
slides which indicate the presenter's progress based on slide count.

This progress bar is not drawn on image slides, since it may clutter
and/or interfere with the figure on the slide.
---
 config.def.h | 3 +++
 sent.c   | 6 ++
 2 files changed, 9 insertions(+)

diff --git a/config.def.h b/config.def.h
index 60eb376..25d89e0 100644
--- a/config.def.h
+++ b/config.def.h
@@ -19,6 +19,9 @@ static const float linespacing = 1.4;
 static const float usablewidth = 0.75;
 static const float usableheight = 0.75;
 
+/* height of the presentation progress bar */
+static const int progressheight = 5;
+
 static Mousekey mshortcuts[] = {
/* button functionargument */
{ Button1,advance,{.i = +1} },
diff --git a/sent.c b/sent.c
index c50a572..046466e 100644
--- a/sent.c
+++ b/sent.c
@@ -533,6 +533,12 @@ xdraw()
 0,
 slides[idx].lines[i],
 0);
+   if (idx != 0 && progressheight != 0) {
+   drw_rect(d,
+0, xw.h - progressheight,
+(xw.w * idx)/(slidecount - 1), progressheight,
+1, 0);
+   }
drw_map(d, xw.win, 0, 0, xw.w, xw.h);
} else {
if (!(im->state & SCALED))
-- 
2.15.1

Re: [hackers] [sbase] [PATCH] ls: allow listing contents of directories with +r-x

[David Phillips]

On Fri, Oct 20, 2017 at 01:28:30PM -0700, Michael Forney wrote:
> Hi David,

Hi Michael

> 
> Can you explain the reasoning behind this? Is this behavior
> standardized, or consistent with other ls(1) implementations?
> 

The purpose for this behaviour was to allow stat to fail when it is
used to find properties of files in a directory. For example:

$ mkdir foo
$ touch foo/bar
$ chmod 600 tmp
$ ls tmp
ls: lstat foo/bar: Permission denied
?-0 root root  0 Jan 01  1970 bar

Being able to at least see the names of the files that are contained
in a directory where we have no execute permission may be useful,
but (l)stat failure normally results in a fatal error for ls.

Regarding how standard this behaviour is, I've not read up on this in
the POSIX standard, but it is the way that another ls implementation
appears to behave (GNU). Thanks for bringing this point up though,
since I just tested on a SunOS machine and found it to simply report
'total 0', something I didn't realise it did.

Additionally, the output in such a case is probably another issue
that I have only just noticed (all zeroes → root:root 1970-01-01).

> > +   if (prefix != NULL)
> > +   snprintf(path, PATH_MAX, "%s/%s", prefix, name);
> 
> You need to check whether this returned an error, or the result was truncated.
> 

You're right. There are many other parts of the code that use snprintf
without error checking. These others might be the topic for another
patch, but I agree that this patch should at least be modified to
check for error.

> > +   else
> > +   strncpy(path, name, PATH_MAX);
> 
> strncpy is almost never what you want. It does not null-terminate the
> string if it doesn't fit in the destination, and if it is shorter than
> PATH_MAX, the remaining buffer will be filled with zeros.
> 

Definitely a good catch here too, I hadn't remembered that limitation
of strncpy. Once the other points are discussed, I will resubmit a
patch that uses strlcpy.


Grateful for your notes,
David

[hackers] [sbase] [PATCH] ls: allow listing contents of directories with +r-x

[David Phillips]

chdir()ing into a directory with +r-x fails, so we should manually use the
directory name as a prefix rather than chdir()ing into it.

Also adds new parameters to mkent for the prefix, and for dictating whether
or not a permission denied error when stat()ing shall be fatal or not. This
allows errors stat()ing children of `foo` to be reported and non-fatal, while
a permission denied error on `foo` itself would be fatal.
---
 ls.c | 33 +++--
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/ls.c b/ls.c
index b716aba..c01a277 100644
--- a/ls.c
+++ b/ls.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "utf.h"
 #include "util.h"
@@ -58,15 +59,29 @@ static int showdirs;
 static void ls(const char *, const struct entry *, int);
 
 static void
-mkent(struct entry *ent, char *path, int dostat, int follow)
+mkent(struct entry *ent, char *prefix, char *name, int dostat, int follow,
+  int fatal)
 {
struct stat st;
+   char path[PATH_MAX];
 
-   ent->name = path;
+   ent->name = name;
if (!dostat)
return;
-   if ((follow ? stat : lstat)(path, &st) < 0)
-   eprintf("%s %s:", follow ? "stat" : "lstat", path);
+
+   if (prefix != NULL)
+   snprintf(path, PATH_MAX, "%s/%s", prefix, name);
+   else
+   strncpy(path, name, PATH_MAX);
+
+   if ((follow ? stat : lstat)(path, &st) < 0) {
+   if (errno == EACCES && !fatal) {
+   weprintf("%s %s:", follow ? "stat" : "lstat", path);
+   return;
+   } else {
+   eprintf("%s %s:", follow ? "stat" : "lstat", path);
+   }
+   }
ent->mode  = st.st_mode;
ent->nlink = st.st_nlink;
ent->uid   = st.st_uid;
@@ -252,8 +267,6 @@ lsdir(const char *path, const struct entry *dir)
weprintf("opendir %s%s:", path, dir->name);
return;
}
-   if (chdir(dir->name) < 0)
-   eprintf("chdir %s:", dir->name);
 
while ((d = readdir(dp))) {
if (d->d_name[0] == '.' && !aflag && !Aflag)
@@ -264,8 +277,8 @@ lsdir(const char *path, const struct entry *dir)
continue;
 
ents = ereallocarray(ents, ++n, sizeof(*ents));
-   mkent(&ents[n - 1], estrdup(d->d_name), Fflag || iflag ||
-   lflag || pflag || Rflag || sort, Lflag);
+   mkent(&ents[n - 1], dir->name, estrdup(d->d_name), Fflag || 
iflag ||
+   lflag || pflag || Rflag || sort, Lflag, 0);
}
 
closedir(dp);
@@ -446,7 +459,7 @@ main(int argc, char *argv[])
case 0: /* fallthrough */
*--argv = ".", ++argc;
case 1:
-   mkent(&ent, argv[0], 1, Hflag || Lflag);
+   mkent(&ent, NULL, argv[0], 1, Hflag || Lflag, 1);
ls("", &ent, (!dflag && S_ISDIR(ent.mode)) ||
(S_ISLNK(ent.mode) && S_ISDIR(ent.tmode) &&
 !(dflag || Fflag || lflag)));
@@ -454,7 +467,7 @@ main(int argc, char *argv[])
break;
default:
for (i = ds = fs = 0, fents = dents = NULL; i < argc; ++i) {
-   mkent(&ent, argv[i], 1, Hflag || Lflag);
+   mkent(&ent, NULL, argv[i], 1, Hflag || Lflag, 1);
 
if ((!dflag && S_ISDIR(ent.mode)) ||
(S_ISLNK(ent.mode) && S_ISDIR(ent.tmode) &&
-- 
2.14.2



signature.asc
Description: PGP signature

Re: [hackers] [sbase] [PATCH 2/2] [chmod] Use DIRFIRST

[David Phillips]

On Sun, Oct 01, 2017 at 12:03:45PM -0700, Michael Forney wrote:
> Here's what POSIX has to say on the matter:
> 
> "Some implementations of the chmod utility change the mode of a
> directory before the files in the directory when performing a
> recursive (-R option) change; others change the directory mode after
> the files in the directory. If an application tries to remove read or
> search permission for a file hierarchy, the removal attempt fails if
> the directory is changed first; on the other hand, trying to re-enable
> permissions to a restricted hierarchy fails if directories are changed
> last. Users should not try to make a hierarchy inaccessible to
> themselves."
> 
> So it looks like we are conforming whether we use DIRFIRST or not.
> However, based on the last sentence, we probably don't expect users to
> `chmod -R 000` a directory tree and DIRFIRST seems to match a couple
> other implementations I looked at, so I applied this patch.
> 
> Thanks!

Yeah, I had identified this tradeoff when making the patch. Unfortunately
it is a mutually exclusive relationship, unless we were to approach it in
a "smarter" manner and examine our gid and uid, the target's gid and uid,
as well as the new mode itself. Then we could try to detect if applying the
fn before calling opendir would result in error. When I first thought of
this method, it sounded a little too sucky for me.

A preliminary version of the patch that I wrote actually "brute forced"
the opendir before calling fn, and if that failed, it also tried calling
it after fn was run, just in case fn made it readable to us. This worked
in that I was able to recursively change trees between extremes such as 000
and 777, but it came with a tradeoff of applying the function twice (on the
way down as well as up), and it just seemed sucky to rely on such a brutish
approach.

If POSIX doesn't specify the behaviour, and our behaviour doesn't suck, I
guess it makes sense for it to match "most" other tools out there. Testing
shows me that both GNU and Solaris versions of chmod seem to act in the same
way as our chmod now does.

Thanks for applying it!
David

[hackers] [sbase] [PATCH 1/2] [libutil/recurse] only opendir if recursing

[David Phillips]

Previous behaviour was to call opendir regardless of if we are actually going
to be recursing into the directory. Additionally, some utilities that use
DIRFIRST benefit from running the function pointed to by fn before the call
to opendir. One such example is `chmod [-R] 777 dir` on a directory with mode
000, where it will be expected for chmod to first give itself rwx before
optionally listing the directory to traverse it.
---
 libutil/recurse.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/libutil/recurse.c b/libutil/recurse.c
index e2b8a6e..38bee42 100644
--- a/libutil/recurse.c
+++ b/libutil/recurse.c
@@ -54,18 +54,17 @@ recurse(const char *path, void *data, struct recursor *r)
if (h->ino == st.st_ino && h->dev == st.st_dev)
return;
 
-   if (!(dp = opendir(path))) {
-   if (!(r->flags & SILENT)) {
-   weprintf("opendir %s:", path);
-   recurse_status = 1;
-   }
-   return;
-   }
-
if (!r->depth && (r->flags & DIRFIRST))
(r->fn)(path, &st, data, r);
 
if (!r->maxdepth || r->depth + 1 < r->maxdepth) {
+   if (!(dp = opendir(path))) {
+   if (!(r->flags & SILENT)) {
+   weprintf("opendir %s:", path);
+   recurse_status = 1;
+   }
+   return;
+   }
while ((d = readdir(dp))) {
if (r->follow == 'H') {
statf_name = "lstat";
-- 
2.14.1

Re: [hackers] [sbase] [PATCH 1/2] [libutil/recurse] only opendir if recursing

[David Phillips]

On Sun, Oct 01, 2017 at 12:35:23AM -0700, Michael Forney wrote:
> Hi David,
> 
> Thanks for the patch. The general idea seems good to me.
> 

Thanks!

> 
> I don't understand the `!r->maxdepth` check here. If `r->depth + 1 <
> r->maxdepth` is used to enter the outer branch, won't an uninitialized
> `dp` to be passed to readdir?
> 

Hmmm… neither do I. The only thing that condition could guard against is
`r->depth < -1`, which is nonsensical. You're definitely right about the
uninitialised `dp` being passed to readdir too.

Looks like I accidentally sent the wrong version of the patch (the version
crafted late night/early morning), especially judging by the broken English
in my commit message. I'll send the updated patch in a wee bit.

Thanks,
David

[hackers] [sbase] [PATCH 2/2] [chmod] Use DIRFIRST

[David Phillips]

Previosuly, running `chmod 777` on a directory that had no read or
execute access (e.g. 111 or 000) would cause chmod to throw its
toys since it was trying to opendir before having added read permission
to the directory.
---
 chmod.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/chmod.c b/chmod.c
index f671f32..c12d2f4 100644
--- a/chmod.c
+++ b/chmod.c
@@ -32,7 +32,7 @@ int
 main(int argc, char *argv[])
 {
struct recursor r = { .fn = chmodr, .hist = NULL, .depth = 0, .maxdepth 
= 1,
- .follow = 'P', .flags = 0 };
+ .follow = 'P', .flags = DIRFIRST };
size_t i;
 
argv0 = *argv, argv0 ? (argc--, argv++) : (void *)0;
-- 
2.14.1

[hackers] [sbase] [PATCH 1/2] [libutil/recurse] only opendir if recursing

[David Phillips]

Previous behaviour was to call opendir regardless of if we are actually
going to be recursing into the direectory. Additionally, some utilities
require like chmod that DIRFIRST causes the fn to execute before the
opendir is called.
---
 libutil/recurse.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/libutil/recurse.c b/libutil/recurse.c
index e2b8a6e..b356d2c 100644
--- a/libutil/recurse.c
+++ b/libutil/recurse.c
@@ -54,18 +54,17 @@ recurse(const char *path, void *data, struct recursor *r)
if (h->ino == st.st_ino && h->dev == st.st_dev)
return;
 
-   if (!(dp = opendir(path))) {
-   if (!(r->flags & SILENT)) {
-   weprintf("opendir %s:", path);
-   recurse_status = 1;
-   }
-   return;
-   }
-
if (!r->depth && (r->flags & DIRFIRST))
(r->fn)(path, &st, data, r);
 
if (!r->maxdepth || r->depth + 1 < r->maxdepth) {
+   if (!r->maxdepth && !(dp = opendir(path))) {
+   if (!(r->flags & SILENT)) {
+   weprintf("opendir %s:", path);
+   recurse_status = 1;
+   }
+   return;
+   }
while ((d = readdir(dp))) {
if (r->follow == 'H') {
statf_name = "lstat";
-- 
2.14.1

Re: [hackers] [sbase][PATCH] ls: abort a directory if we cannot opendir it

[David Phillips]

Sorry, I should have made the commit message clearer. It is
not the entire operation that is being aborted, but only the
current node, if it cannot be opened.

Before this patch, ls will segfault since it tries to perform
operations on a (null) DIR*. After this patch, it prints the
message, sets the return code and doesn't segfault.

I found this bug while playing with the -R flag (though it is not
isolated to this flag), and I have confirmed that behaviour is
correct as you describe. It will do the regular recursive listing
for all the heirarchy, and in amongst its output is the

ls: opendir ./foodir: Permission denied

output.

Apologies again if my commit message was a bit ambiguous on what
exactly was being aborted.

All the best,
David

On Tue, Aug 22, 2017 at 10:08:48AM +0200, Quentin Rameau wrote:
> Hello David,
> 
> > We should not try and perform operations on an invalid DIR* stream.
> > Instead, we shall let the error message be printed, and the return
> > code set (existing behaviour) and abort afterwards.
> 
> Any justification you could provide us with?
> 
> AFAIK POSIX specifies the opposite, a diognostic message should be
> issued on stderr and the utility should continue processing the
> remaining of the operands/hierarchy.
> 
> > ---
> >  ls.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/ls.c b/ls.c
> > index 5080c8f..b716aba 100644
> > --- a/ls.c
> > +++ b/ls.c
> > @@ -250,6 +250,7 @@ lsdir(const char *path, const struct entry *dir)
> > if (!(dp = opendir(dir->name))) {
> > ret = 1;
> > weprintf("opendir %s%s:", path, dir->name);
> > +   return;
> > }
> > if (chdir(dir->name) < 0)
> > eprintf("chdir %s:", dir->name);
> 

[hackers] [sbase][PATCH] ls: abort a directory if we cannot opendir it

[David Phillips]

We should not try and perform operations on an invalid DIR* stream.
Instead, we shall let the error message be printed, and the return
code set (existing behaviour) and abort afterwards.
---
 ls.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ls.c b/ls.c
index 5080c8f..b716aba 100644
--- a/ls.c
+++ b/ls.c
@@ -250,6 +250,7 @@ lsdir(const char *path, const struct entry *dir)
if (!(dp = opendir(dir->name))) {
ret = 1;
weprintf("opendir %s%s:", path, dir->name);
+   return;
}
if (chdir(dir->name) < 0)
eprintf("chdir %s:", dir->name);
-- 
2.14.1



signature.asc
Description: PGP signature

Re: [hackers] Desktop icons in dwm

[David Phillips]

On Wed, Apr 05, 2017 at 07:09:50AM +, Cág wrote:
> [...] 
> 

Maybe a late April 1st submission?

Re: [hackers] Re: Digest of hackers@suckless.org issue 367 (10860-10886)

[David Phillips]

Replied off-list with help/instructions.

Re: [hackers] [st] Enable login shell?

[David Phillips]

On Tue, Nov 08, 2016 at 04:41:27PM -0500, lo...@lorentrogers.com wrote:
>
> This should be documented somewhere--I'll see if the RVM folks can
> include a note on st in their docs.
>

Hello,

The `-e foocmd` behaviour is fairly common amongst many different
terminal emulators; it is not something that is exclusive to st.

Cheers


signature.asc
Description: PGP signature

[hackers] [sent] [PATCH] Check return value of calloc in ffload

[David Phillips]

---
 sent.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sent.c b/sent.c
index 82acd81..1518015 100644
--- a/sent.c
+++ b/sent.c
@@ -210,7 +210,9 @@ ffload(Slide *s)
if (memcmp("farbfeld", hdr, 8))
die("sent: Filtered file '%s' has no valid farbfeld header", 
filename);
 
-   s->img = calloc(1, sizeof(Image));
+   if (!(s->img = calloc(1, sizeof(Image
+   die("sent: Unable to allocate image:");
+
s->img->bufwidth = ntohl(*(uint32_t *)&hdr[8]);
s->img->bufheight = ntohl(*(uint32_t *)&hdr[12]);
 
-- 
2.10.2



signature.asc
Description: PGP signature

Re: [hackers] [dwm][PATCH] Do not call die() upon '-v' invocation

[David Phillips]

On Fri, Oct 28, 2016 at 03:58:14PM +0200, Martin Kühne wrote:
> IMHO, when failing to parse command line arguments, usage() should be
> called before exiting with EXIT_FAILURE.
> on invocation with -h|--help, it should exit with EXIT_SUCCESS.
> 
> cheers!
> mar77i

This sounds sane and solves the problems from both sides of the camp.

Re: [hackers] [dwm][PATCH] Corrected typo.

[David Phillips]

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.


signature.asc
Description: PGP signature

Re: [hackers] [patch] st crash inside getsel

[David Phillips]

On Tue, Apr 19, 2016 at 02:48:14AM +0100, ra...@openmailbox.org wrote:
> Hello
> 
> I had been noticing st 0.6 terminals sometimes disappearing on openbsd. I
> managed to reproduce the crash quite consistently by trying to select and
> copy the output of: man ls | head -10.
> 
> Debugged the coredump and saw that linelen is always zero before the crash -
> in which case I think there is an underflow in this line: last =
> &term.line[y][MIN(lastx, linelen-1)]; that was resulting in memory
> corruption which caused the subsequent last->mode to segment fault.
> 
Hi,

Looks like k0ga fixed this in commit 5d2d9d54, in September last year.

Thanks


signature.asc
Description: PGP signature

Re: [hackers] [slock][PATCH] Reset color to INIT on Escape key press

[David Phillips]

On Mon, Apr 04, 2016 at 02:46:18AM -0300, Thomas wrote:
> Also makes sense because pushing Esc isn't really a failure, but a
> purposefully aborted unlock attempt, so the background shouldn't be set
> to FAILURE.

The main reason for the inclusion of the 'fail on clear' behaviour was so that
you could see if anyone tampered with the computer while it was locked:

 "while I was out getting a cup of tea, did I hear someone typing on my
  keyboard or was it just the paranoia pixies?"

This is a common problem for users of slock. Fail on clear addressed the issue.
Please note also that pressing backspace to empty the input buffer will result
in the failure colour being shown even though this "isn't really a failure"
either :)

In order to get the behaviour you're after, is there a problem with simply
setting failonclear to False in config.h? I understand the behaviour isn't
identical to your patch, but the "security" is the same. With this patch
applied, if Mallory failed to guess your password, he can just press Esc and
you're none the wiser.

> (Note: let me know if you prefer to pull from Github, I'm new to the
> list.)

Email+patch is the way to go.


signature.asc
Description: PGP signature

Re: [hackers] [sent] [PATCH 2/2] replace farbfeld with libnetpbm

[David Phillips]

On Sun, Dec 13, 2015 at 06:40:27AM +0100, FRIGN wrote:
> 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.

I won't personally give an argument, I feel like farbfeld is nice and
standard and fits this purpose a little better than netpbm.

>From my perspective, netpbm has too many extensions and "flavours" to
provide support without an external library.

[hackers] [sent] [PATCH] Change an eprintf to a die to stop child from running its own slideshow

[David Phillips]

The child thread was created because execlp will not return if successful.
The eprintf was placed after the call to execlp to catch any error, but the
child continued to return a (closed) fds[0], resulting in a second slideshow
being run by the child.

This commit fixes the issue by killing the child.
---
 sent.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sent.c b/sent.c
index fc5e389..88abe90 100644
--- a/sent.c
+++ b/sent.c
@@ -153,7 +153,7 @@ filter(int fd, const char *cmd)
close(fds[0]);
close(fds[1]);
execlp(cmd, cmd, (char *)0);
-   eprintf("execlp %s:", cmd);
+   die("execlp %s:", cmd);
}
close(fds[1]);
return fds[0];
--
2.6.2

Re: [hackers] Compiling stali

[David Phillips]

Did you read the error yourself?

Software does not output errors without reason.

Re: [hackers] [st] startup options

[David Phillips]

Me, I use dvtm(1) instead of tmux, and I only have my dwm config set
to run st with a dvtm session inside. This way, I can manually start
st without dvtm by asking for `st` from dmenu or another terminal. I
suppose it's a matter of personal taste, but when I run 'st' I don't
want dvtm or tmux. If I want them I'll ask for them by specifying such
(but I know this goes against the OP's question), or by pressing the
appropriate dwm hotkey

This feature may rub some people up the wrong way, but it's just more
food for thought.

Re: [hackers] [PATCH] [sent] Quick patch to replace png with farbfeld

[David Phillips]

> Can sent just use .ff.bz2 to display images?

Going by the current proposal, you would just tell sent to use bzcat
to get farbfeld data from a .ff.bz2, just like telling it to use
png2ff to get farbfeld data from a PNG.

[hackers] Re: Patch naming on the wiki

[David Phillips]

Ah bollocks, wrong list, should be dev@

[hackers] Patch naming on the wiki

[David Phillips]

Just wondering what the rest of the community reckons about an issue
that popped up briefly on IRC.

I'll start with an example: some patches, for a long time, have been
named `dwm-6.1-fibwibble.diff`—long before dwm-6.1 was released. When
I was more of a newbie, this was confusing. Now, though, I do
understand that it was really just a patch against master "in
anticipation" of the next version/tag.

It all seems fine before this version is released, because "everyone
knows 6.1 isn't out yet," so it "must be a patch against master." It
gets confusing though, because some patches stop being updated to the
latest master, so when the new version actually arrives, these patches
which look like they will apply, whereas they are actually patches
against some old ref, perhaps from months or years ago

What I'm proposing, or rather asking for the community opinion on, is
whether or not we continue naming (what are really git master) patches
like this.

What I propose is what a lot of patches already use, which is:

* foo-[short commit hash].diff
* foo-MMDD.diff
* foo--MM-DD.diff

or a combination/mish-mash of the above.

(It might be good to settle on the (standard) -MM-DD rather than
MMDD in this discussion as well)

What do you all think?

Re: [hackers] [surf] [PATCH 2/2] style fixes: space after keywords, () with sizeof

[David Phillips]

Looking back at it [1] now, I see it doesn't put brackets with sizeof.
I feel that this would require full-on parsing to do correctly.
Nevertheless, it does a lot of the heavy-lifting and I'll speculate
that sizeof without brackets is a lot less common than keywords
without spaces (ones which should have them).

I also don't want to make any claims about its safety; I'm not sure
there isn't a special case out there where it would fail. However it
has received some moderate testing and appears to work fine. Let me
know what you all think.

Thanks

[1] https://ptpb.pw/FN5_

Re: [hackers] [surf] [PATCH 2/2] style fixes: space after keywords, () with sizeof

[David Phillips]

> Thanks. Now will you update all the patches that broke?

Honestly? This could is trivial to automate. I wrote a script a while
back which did just this.

[hackers] [slock] [PATCH] Don't change to failure colour on success

[David Phillips]

As it stands, the success behaviour for slock is to change to the
failure colour and exit, since the string length of the password field
is set to zero. This flash of failure colour isn't normally noticed,
but it becomes an issue when a composite manager is used and window
fading is used.

This patch stops slock from changing the colour of the window after
success, so the input colour is seen right up until exit.

Thanks
diff --git a/slock.c b/slock.c
index b3bee92..6be8f22 100644
--- a/slock.c
+++ b/slock.c
@@ -187,7 +187,7 @@ readpw(Display *dpy, const char *pws)
break;
}
color = len ? INPUT : (failure || failonclear ? FAILED 
: INIT);
-   if (oldc != color) {
+   if (running && oldc != color) {
for (screen = 0; screen < nscreens; screen++) {
XSetWindowBackground(dpy, 
locks[screen]->win, locks[screen]->colors[color]);
XClearWindow(dpy, locks[screen]->win);

Re: [hackers] [abduco] [PATCH] Allow squashing of command line flags together

[David Phillips]

Here is the getopt patch as requested. Much tidier, and I haven't
found any issues yet with some decent testing.
diff --git a/abduco.c b/abduco.c
index 5c4d174..17a1896 100644
--- a/abduco.c
+++ b/abduco.c
@@ -545,34 +545,25 @@ static int list_session(void) {
 
 int main(int argc, char *argv[]) {
bool force = false;
+   int c = 0;
+   char *esc = NULL;
char **cmd = NULL, action = '\0';
server.name = basename(argv[0]);
gethostname(server.host+1, sizeof(server.host) - 1);
if (argc == 1)
exit(list_session());
-   for (int arg = 1; arg < argc; arg++) {
-   if (argv[arg][0] != '-') {
-   if (!server.session_name) {
-   server.session_name = argv[arg];
-   continue;
-   } else if (!cmd) {
-   cmd = &argv[arg];
-   break;
-   }
-   }
-   if (server.session_name)
-   usage();
-   switch (argv[arg][1]) {
+
+   while ((c = getopt(argc, argv, "aAcne:frv")) != -1)
+   {
+   switch (c) {
case 'a':
case 'A':
case 'c':
case 'n':
-   action = argv[arg][1];
+   action = c;
break;
case 'e':
-   if (arg + 1 >= argc)
-   usage();
-   char *esc = argv[++arg];
+   char *esc = optarg;
if (esc[0] == '^' && esc[1])
*esc = CTRL(esc[1]);
KEY_DETACH = *esc;
@@ -591,6 +582,17 @@ int main(int argc, char *argv[]) {
}
}
 
+
+   /* collect the session name if trailing args */
+   if (optind < argc)
+   server.session_name = argv[optind];
+
+
+   /* if yet more trailing arguments, they must be the command */
+   if (optind + 1 < argc)
+   cmd = &argv[optind + 1];
+
+
if (!cmd) {
cmd = (char*[]){ getenv("ABDUCO_CMD"), NULL };
if (!cmd[0])

Re: [hackers] [abduco] [PATCH] Allow squashing of command line flags together

[David Phillips]

Yeah you'd be right about having issues with -e, I must've missed that.

I'll submit the getopt-based one soon once it's polished off.

[hackers] [abduco] [PATCH] Allow squashing of command line flags together

[David Phillips]

Currently, abduco only supports command line flags kept separate, eg
`abduco -c -f foo bar`. This patch changes the parser to treat `-abc`
as being equal to `-a -b -c`, effectively allowing options to be
'squashed' together. This would mean `abduco -c -f foo bar` could
equally be expressed as `abduco -cf foo bar`.

As an additional note, I had a play around with switching to getopt
rather than reinventing the wheel as abduco currently does. I could
submit that patch also/instead if it would be preferred over this one.
diff --git a/abduco.c b/abduco.c
index 5c4d174..77efa62 100644
--- a/abduco.c
+++ b/abduco.c
@@ -562,32 +562,35 @@ int main(int argc, char *argv[]) {
}
if (server.session_name)
usage();
-   switch (argv[arg][1]) {
-   case 'a':
-   case 'A':
-   case 'c':
-   case 'n':
-   action = argv[arg][1];
-   break;
-   case 'e':
-   if (arg + 1 >= argc)
+   int c = 0;
+   while(argv[arg][++c]) {
+   switch (argv[arg][c]) {
+   case 'a':
+   case 'A':
+   case 'c':
+   case 'n':
+   action = argv[arg][c];
+   break;
+   case 'e':
+   if (arg + 1 >= argc)
+   usage();
+   char *esc = argv[++arg];
+   if (esc[0] == '^' && esc[1])
+   *esc = CTRL(esc[1]);
+   KEY_DETACH = *esc;
+   break;
+   case 'f':
+   force = true;
+   break;
+   case 'r':
+   client.readonly = true;
+   break;
+   case 'v':
+   puts("abduco-"VERSION" © 2013-2015 Marc André 
Tanner");
+   exit(EXIT_SUCCESS);
+   default:
usage();
-   char *esc = argv[++arg];
-   if (esc[0] == '^' && esc[1])
-   *esc = CTRL(esc[1]);
-   KEY_DETACH = *esc;
-   break;
-   case 'f':
-   force = true;
-   break;
-   case 'r':
-   client.readonly = true;
-   break;
-   case 'v':
-   puts("abduco-"VERSION" © 2013-2015 Marc André Tanner");
-   exit(EXIT_SUCCESS);
-   default:
-   usage();
+   }
}
}
 

[hackers] Re: [slock] [PATCH] Slightly safer OOM killer disablement

[David Phillips]

Resubmitting after talking with the guys in IRC, and after noticing
that whitespace got messed with in my original patch.

-- 
Four word witty remark
diff --git a/slock.c b/slock.c
index d6053af..b3bee92 100644
--- a/slock.c
+++ b/slock.c
@@ -60,16 +60,27 @@ die(const char *errstr, ...)
 
 #ifdef __linux__
 #include 
+#include 
 
 static void
 dontkillme(void)
 {
int fd;
+   int length;
+   char value[64];
 
fd = open("/proc/self/oom_score_adj", O_WRONLY);
if (fd < 0 && errno == ENOENT)
return;
-   if (fd < 0 || write(fd, "-1000\n", 6) != 6 || close(fd) != 0)
+
+   /* convert OOM_SCORE_ADJ_MIN to string for writing */
+   length = snprintf(value, sizeof(value), "%d\n", OOM_SCORE_ADJ_MIN);
+
+   /* bail on truncation */
+   if (length >= sizeof(value))
+   die("buffer too small\n");
+
+   if (fd < 0 || write(fd, value, length) != length || close(fd) != 0)
die("cannot disable the out-of-memory killer for this 
process\n");
 }
 #endif

[hackers] [slock] [PATCH] Slightly safer OOM killer disablement

[David Phillips]

The old behaviour of slock was to write the hardcoded string "-1000\n"
to `/proc/self/oom_score_adj`. My patch, while a bit less tidy, errs
on the side of caution by deriving this string from OOM_SCORE_ADJ_MIN,
which is defined in `linux/oom.h` as being the score to use to disable
the OOM killer.

I'd be interested in any feedback on this patch, as I feel bad for
submitting a patch which makes code less tidy :-P

-- 
Four word witty remark
diff --git a/slock.c b/slock.c
index d6053af..8510df0 100644
--- a/slock.c
+++ b/slock.c
@@ -60,16 +60,25 @@ die(const char *errstr, ...)
 #ifdef __linux__
 #include 
+#include 
 static void
 dontkillme(void)
 {
-   int fd;
+   int fd, length;
+   char value[64];
fd = open("/proc/self/oom_score_adj", O_WRONLY);
if (fd < 0 && errno == ENOENT)
return;
-   if (fd < 0 || write(fd, "-1000\n", 6) != 6 || close(fd) != 0)
+
+   /* convert OOM_SCORE_ADJ_MIN to string for writing */
+   if (snprintf(value, sizeof(value), "%d\n", OOM_SCORE_ADJ_MIN) >= 
sizeof(value))
+   die("cannot convert OOM_SCORE_ADJ_MIN to string of less than %d 
bytes\n", sizeof(value));
+
+   length = strlen(value);
+
+   if (fd < 0 || write(fd, value, length) != length || close(fd) != 0)
die("cannot disable the out-of-memory killer for this 
process\n");
 }
 #endif