Re: [hackers] [slock] [PATCH] Refactor main()

From: Dimitris Papastamos <sin_AT_2f30.org>
Date: Mon, 22 Aug 2016 15:48:47 +0100

On Mon, Aug 22, 2016 at 04:17:31PM +0200, FRIGN wrote:
> On Mon, 22 Aug 2016 12:32:28 +0200
> Markus Teich <markus.teich_AT_stusta.mhn.de> wrote:
>
> > 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.

The project code itself is the reference, avoid unnecessary changes.

> > > + /* 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.

I would go as far as not introducing any header file and moving the
explicit_bzero declaration in slock.c. When and if more wrappers are required
the declarations can be factored out into a separate header file.
Received on Mon Aug 22 2016 - 16:48:47 CEST