On Mon, 22 Aug 2016 12:32:28 +0200
Markus Teich <markus.teich_AT_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
> > _AT_@ -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 <dev_AT_frign.de>
Received on Mon Aug 22 2016 - 16:17:31 CEST