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>
