Re: avltree / genalloc

2018-04-13 Thread Olivier Brunel

>   Then genalloc is indeed the right data structure from you, but
> I don't have helpers to arbitrarily remove an object in the middle.
> Note that if you don't need index stability, it's trivial, and O(1):
> you can just overwrite the ith object with the (n-1)th, and decrement
> the length. No need to copy len-i-1 objects, just copy one.

Ah, yes. But when I said I don't need index stability, I meant current
4 can become 2, but I actually want to preserve the order of elements
within the array... hence moving the whole data block.
(I guess for a generic genalloc_delete_size an extra flag indicating
whether to move all elements or just move the last one might be good.)

Cheers,


Re: avltree / genalloc

2018-04-12 Thread Olivier Brunel


Many thanks for the explaination/clarifications!


>   avltree_iter isn't meant to perform operations that modify the 
> structure
> of the tree. It *may* work, but there's no guarantee it will - and
> typically, if you modify the tree, later values for node height will
> certainly be incorrect. I wrote avltree_iter as a shortcut to perform
> simple operations such as printing the tree or calling a function on
> the data contained in every node; it should not be used with a
> tree-modifying function.

Ok, so let me ask you this: I want to delete my avltree. That is, I'd
like to remove all its nodes and be done with it, i.e. I need to free
all memory associated with it. I guess avltree_free() is enough, no
need to delete each node individually?

Obviously I want to do the same with my data, stored in a gensetdyn,
but there gensetdyn_free isn't enough, because I have extra memory
allocated (on a per-item basis) that I need to free as well, hence I
need to iterate over the items to perform my own freeing.

So yeah, that should in fact be done over the gensetdyn not the
avltree, that was my mistake, and then I simply not delete anything
either, just free my memory and call gensetdyn_free is enough, correct?

(Just to know, would the same be true with gensetdyn, and I shouldn't
add/remove during a gensetdyn_iter?)


> future major release - and use avltree_delete(), which is the right
> API. And the argument to avltree_delete() is the data in the node,
> i.e. your uint32_t (that is probably the index of your object in your
> gensetdyn).

Just to be clear, avltree_delete() actually takes the key (void *) for
that data, not the uint32_t itself, right?


>   Well, genalloc is really "stralloc for larger objects", and I try
> not to implement for genalloc what I would not implement for stralloc.
> If you often need to remove items from a genalloc that are not the
> last one, then genalloc may not be the appropriate data structure,
> and you may want a gensetdyn instead.

hmm... I've been thinking about this, but I feel like what I need is
just a (dynamic) array, that I can add to and remove from. Maybe I'm
not seeing a genalloc exactly same as you; I feel gensetdyn might be
good for that, but I don't actually need indices to be stable (I'm
storing pointers), which is why a genalloc seems right to me: it's an
array, that can grow (or shrink) as needed. No need for more complexity
or anything. genalloc is pretty simple & straightforward, it's good,
but I do need to remove items from time to time, and that means I need
the memory moving bit.


Cheers,


avltree / genalloc

2018-04-12 Thread Olivier Brunel
Hey,

So I've been using avltree & gensetdyn (amongst others) lately in a
little project of mine, and I have some questions:

- When using avltree_iter() we need to pass a avliterfunc_t_ref, which
  takes 3 arguments. If I understand correctly, first is the uint32_t
  index, then the tree's "internal" (unsigned int) index, and my
  pointer.

  So if I wanted, from there, to remove an item from the avltree, I
  could either use avltree_deletenode(tree, ARG2) or
  avltree_delete(tree, KEY) where KEY is the one from dtok(ARG1,p) --
  would all that be correct, or did I miss/misunderstood something?

- So it means avltree_deletenode() needs the avltree's own/internal
  index, whereas gensetdyn_delete() needs "our" uint32_t index,
  correct?
  I.e. if I have that uint32_t and want to remove things from both the
  avltree & gensetdyn (I'm using the later to store the actual data
  indexed via the former, btw) then for avltree I need to get the key
  (e.g. via t->dtok(u,p))

- I think there's a bug in skalibs right now, avltree_deletenode() is
  broken and things don't even compile; I believe the attached patch is
  a correct fix for it.
  (I ended up using avltree_delete directly, guessing you usually do
  that as well maybe?)


- And on a completely unrelated topic, is there really no way
  (macro/function) in skalibs to remove an item from a genalloc? I get
  it for stralloc, it's not really thought of as an array but a string,
  you rarely want to remove one char from somehere in the middle, whatever.

  But with a genalloc, I'd expect one to (often) be wanting to remove an
  item from the array, and not always/only the last one, yet I can't see
  how that's supposed to be done? (Again, as in via a macro or
  something, of course one can - as I do - handle the memmove oneself)

  Maybe I'm the odd one for wanting to do such a thing on a regular
  basis, but something like in the (other) attached patch, I feel,
  might be useful. I know it certainly wasn't the first time I have to
  write that sort of things...


Thanks for the help!

Cheers,
>From c9d5bb194408f22db8ef859203f28c6748e78ab9 Mon Sep 17 00:00:00 2001
From: Olivier Brunel 
Date: Thu, 12 Apr 2018 18:05:14 +0200
Subject: [PATCH] Fix avltree_deletenode

Signed-off-by: Olivier Brunel 
---
 src/include/skalibs/avltree.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/include/skalibs/avltree.h b/src/include/skalibs/avltree.h
index 1ce7385..8234d95 100644
--- a/src/include/skalibs/avltree.h
+++ b/src/include/skalibs/avltree.h
@@ -48,7 +48,7 @@ extern int avltree_newnode (avltree *, uint32_t, uint32_t *) ;
 #define avltree_insertnode(t, i) avltree_setroot(t, avlnode_insertnode(avltree_nodes(t), avltree_totalsize(t), avltree_root(t), i, (t)->dtok, (t)->kcmp, (t)->external))
 extern int avltree_insert (avltree *, uint32_t) ;
 
-#define avltree_deletenode(t, i) avltree_delete(t, (*(t)->dtok)(avltree_data(t, i)))
+#define avltree_deletenode(t, i) avltree_delete(t, (*(t)->dtok)(avltree_data(t, i),(t)->external))
 extern int avltree_delete (avltree *, void const *) ;
 
 #define avltree_iter(t, f, p) avlnode_iter(avltree_nodes(t), avltree_totalsize(t), avltree_root(t), f, p)
-- 
2.15.1

>From 0c8186bfedc2dfa6d1bd083dafc3ec9a7c46828d Mon Sep 17 00:00:00 2001
From: Olivier Brunel 
Date: Thu, 12 Apr 2018 18:51:40 +0200
Subject: [PATCH] Add genalloc_delete

Signed-off-by: Olivier Brunel 
---
 src/include/skalibs/genalloc.h   |  2 ++
 src/libstddjb/genalloc_delete_size.c | 12 
 2 files changed, 14 insertions(+)
 create mode 100644 src/libstddjb/genalloc_delete_size.c

diff --git a/src/include/skalibs/genalloc.h b/src/include/skalibs/genalloc.h
index da83c2f..acf2bec 100644
--- a/src/include/skalibs/genalloc.h
+++ b/src/include/skalibs/genalloc.h
@@ -28,6 +28,8 @@ typedef stralloc genalloc, *genalloc_ref ;
 #define genalloc_reverse(type, g) stralloc_reverse_blocks((g), sizeof(type))
 #define genalloc_insertb(type, g, offset, s, n) stralloc_insertb((g), (offset)*sizeof(type), (char const *)(s), (n)*sizeof(type))
 #define genalloc_insert(type, g1, offset, g2) stralloc_insert((g1), (offset)*sizeof(type), (g2))
+#define genalloc_delete(type,g,i) genalloc_delete_size((g),sizeof(type),i)
+extern void genalloc_delete_size (genalloc *, size_t, size_t) ;
 
 #define genalloc_deepfree(type, g, f) genalloc_deepfree_size(g, (freefunc_t_ref)(f), sizeof(type))
 extern void genalloc_deepfree_size (genalloc *, freefunc_t_ref, size_t) ;
diff --git a/src/libstddjb/genalloc_delete_size.c b/src/libstddjb/genalloc_delete_size.c
new file mode 100644
index 000..a289000
--- /dev/null
+++ b/src/libstddjb/genalloc_delete_size.c
@@ -0,0 +1,12 @@
+/* ISC license. */
+
+#include 
+#include 
+#include 
+
+void genalloc_delete_size (genalloc *g, size_t s, size_t i)
+{
+  size_t len = g->len / s;
+  if (len > i + 1) memmove (g->s + i * s, g->s + (i + 1) * s, (len - i - 1) * s) ;
+  g->len -= s ;
+}
-- 
2.15.1



Re: [announce] mdevd-0.0.1.0 - a mdev-compatible uevent manager

2018-01-15 Thread Olivier Brunel

I actually already have that trick used to log all events, so I can
tell you that since I booted I've had 640 events processed.

Just ran mdevd-coldplug again, and now I have 1172 events logged, so
that's 532 events generated by mdevd-coldplug just now.

In my log I have basically the output of "date && env && echo ---" and
based on said log, those 532 events are around 120 KB. As for the
640 original events, around 140 KB.

It seems it should all be under the 192 KB of the default buffer, but
as observed with that buffer mdevd runs out of space...

1 MB is propably too much, I should try to see how much is actually
needed. Next time I get to do some reboots I'll see what I can find.

Cheers,


Re: [announce] mdevd-0.0.1.0 - a mdev-compatible uevent manager

2018-01-15 Thread Olivier Brunel
On Mon, 15 Jan 2018 11:39:19 +
"Laurent Bercot"  wrote:

>   The change is obviously incompatible with mdevd-0.0.1.0. It appears
> to work for me, but please test it - especially Olivier who was
> missing events with the old coldplug. If no problems are found, I'll
> cut the 0.1.0.0 release.

The good news is, it appears to work fine here as well, even with my
fancy hardware :p

As a side note however, note that when I updated I also reverted to the
default buffer size (which I noticed you upped), but then not only did
my audio card not show up (amongst other things, this one is noticable
because of a service waiting for it...) but some hard disks seemed not
to have been processed either.
A quick look in the log showed my old friend was back:

mdevd: fatal: unable to receive netlink message: No buffer space
available


And to answer your question, about what could cause such a burst of
events, this time I have an easy answer: mdevd-coldplug !

My first try when I noticed the error was simply to re-run
mdevd-coldplug, and I got the buffer error again (and still no
audio/etc)
Easy fix, I upped the buffer back to 1MB and everything went fine this
time. Rebooted since, with the buffer set to 1MB, and everything works
fine right away. So yay (but my fancy hardware requires a
"larger-than-normal" buffer for mdevd it seems).


Anyhow, it all works fine now, with mdevd & mdevd-coldplug.

Thanks!


Re: [announce] mdevd-0.0.1.0 - a mdev-compatible uevent manager

2018-01-12 Thread Olivier Brunel
On Thu, 11 Jan 2018 23:29:45 -0300
Guillermo  wrote:

> #/bin/execlineb -P
> elglob sysclass /sys/class/*/*/uevent
> elglob sysbus /sys/bus/*/devices/*/uevent
> forx f { $sysclass $sysbus }
> importas -u ueventfile f
> redirfd -w 1 $ueventfile
> echo add

Just tried this, and it worked fine; All my devices were properly
processed. So this might be the way to go indeed, instead of a full
scan of /sys

Cheers,


Re: [announce] mdevd-0.0.1.0 - a mdev-compatible uevent manager

2018-01-10 Thread Olivier Brunel
On Wed, 10 Jan 2018 00:35:33 +
"Laurent Bercot"  wrote:

>   Question that may help:
>   - does your /sys/devices/pci:00/:00:01.0/:01:00.1
> directory contain a "dev" file: before you manually trigger the add?
> afterwards?

Nope, afraid not; neither before nor after. What happens is that, in
each of the case I tried (usb mouse, audio card, pc speaker), after I
write "add" to the uevent file, a new directory will have appeared in
the folder, e.g. mouse0 or input/input5/event5, which will be the one
containing a dev file. (Of course by then there's also a corresponding
syslink in /sys/dev/char).

Sorry, not sure how to avoid using a big hammer...


Cheers,


Re: [announce] mdevd-0.0.1.0 - a mdev-compatible uevent manager

2018-01-09 Thread Olivier Brunel

First off, about the buffer issue, I'm not sure why but I couldn't
reproduce it. :/ Rebooted a few times, with the default buffer, and
mdevd-netlink never complained.
I still have the old messages in my logs, it happened every single
reboot, and not once this time don't know. Maybe I'll try again
later, maybe it's the weather... :D
(I even tried rebooting w/out strace/tee in case, same thing. Damn, I
don't get it, why do things work? uh?... :p)


On Tue, 09 Jan 2018 13:36:48 +
"Laurent Bercot"  wrote:

> >By that I mean, currently if I don't do anything else, some things
> >aren't processed. For instance, my audio card isn't there, neither
> >are a few input devices, such as my (usb) mouse.  
> 
>   I can't reproduce the problem on my system: mdevd-coldplug detects
> all the USB devices. But it's a small system with no fancy hardware.

Not sure I have fancy hardware, but who knows :p

% uname -a
Linux arch.local 4.14.11-1-ARCH #1 SMP PREEMPT Wed Jan 3 07:02:42 UTC
2018 x86_64 GNU/Linux

Not an old kernel, and no firmware involved. What I found out is that,
there are no symlinks in /sys/dev at first no. I'll use my audio card as
example.

So upon boot, nothing in /sys/dev points to it. But if I write "add"
to /sys/devices/pci:00/:00:01.0/:01:00.1/uevent then magic
happens, uevents are triggered, modules loaded, and now my audio card is
there. Also symlinks of related devices do appear in /sys/dev/char:

ls -lR /sys/dev/char|grep 01:00.1
lrwxrwxrwx 1 root root 0 Jan  9 19:58 116:2
-> ../../devices/pci:00/:00:01.0/:01:00.1/sound/card1/controlC1
lrwxrwxrwx 1 root root 0 Jan  9 19:58 116:3
-> ../../devices/pci:00/:00:01.0/:01:00.1/sound/card1/pcmC1D3p
lrwxrwxrwx 1 root root 0 Jan  9 19:58 116:4
-> ../../devices/pci:00/:00:01.0/:01:00.1/sound/card1/hwC1D0
lrwxrwxrwx 1 root root 0 Jan  9 19:58 13:67
-> ../../devices/pci:00/:00:01.0/:01:00.1/sound/card1/input3/event3

Thing is, before writing to that uevent, there was nothing of the sort
in /sys/dev.


% pwd
/sys/devices/pci:00/:00:01.0/:01:00.1

% cat uevent
DRIVER=snd_hda_intel
PCI_CLASS=40300
PCI_ID=1002:AA28
PCI_SUBSYS_ID=1462:AA28
PCI_SLOT_NAME=:01:00.1
MODALIAS=pci:v1002dAA28sv1462sdAA28bc04sc03i00

% ls -l subsystem
lrwxrwxrwx 1 root root 0 Jan  9 20:07 subsystem -> ../../../../bus/pci



I see similar results for e.g. my mouse, not there (or in /dev/input)
until I write "add"
to 
/sys/devices/pci:00/:00:1d.2/usb8/8-1/8-1:1.0/0003:045E:0040.0001/input/input1/uevent;
or the pc speaker, not there until I write "add"
to /sys/devices/platform/pcspkr/uevent, only then does /sys/dev/char/13:66 
(symlink
  to ../../devices/platform/pcspkr/input/input2/event2) appear.


If that's any consolation, `mdev -s` doesn't do any better than
mdevd-coldplug, so maybe it works as expected, and I do have fancy
hardware after all.
(I can't remember if I had tried it before or not, but maybe that was
why I didn't use it, and used my own trigger-uevents service instead.)

Note that I don't mind just using my trigger-uevents service instead,
it's what I used to do anyway; I just figured, as I switched to mdevd,
since there's a coldplug thing might as well give it a try...


Cheers,


Re: [announce] mdevd-0.0.1.0 - a mdev-compatible uevent manager

2018-01-08 Thread Olivier Brunel
On Mon, 08 Jan 2018 19:31:17 +
"Laurent Bercot"  wrote:

> >But, it seems to me that when you're talking about this you expect
> >one to have two different instances of mdevd, one for netlinkd &
> >another one
> >for coldplug. That's not how I made things however: mdevd is a
> >longrun and I simply have both coldplug & netlinkd piped into it.  
> 
>   That can work, but I think it's overly complex.

Well, I don't find it overly complex. It's just two services piped into
the same third one, nothing I find too complex. It's certainly not the
first time I would get different services piped into another common
one. (Even if I don't have helpers as in s6-rc, it's easy enough to do
as well.)


>   One is long-lived, reading from mdevd-netlink, and can be logged ;
> if you want to both supervise it separately from mdevd-netlink and
> log its output, you need a supervision architecture that can pipeline
> more than two longruns. (I wonder where that can be found. ;))
>   The other one is short-lived, reading from mdevd-coldplug which is a
> short-lived program. So you would just have a "mdevd-coldplug | mdevd"
> oneshot running after the mdevd-netlink pipeline has been launched.
> The second mdevd will simply die when the coldplug is over.

Right. I wasn't too worried about running two instances of mdevd at
once, it's just that I like it better when I have the one mdevd
service, and anything it has to say gets logged in the same place,
always, no matter the "source" of the event.

Otherwise it's either the log from mdevd, or coldplug, depending on the
source. But it's not a big deal. I'll see whether I keep things as I
have them, or make mdevd-coldplug piped into its own mdevd.


> >On another topic of sorts: so I start mdevd, netlinkd, and run 
> >coldplug.
> >Cool. But what else would I need to do (upon boot) to ensure
> >everything has been processed correctly?
> >
> >By that I mean, currently if I don't do anything else, some things
> >aren't processed. For instance, my audio card isn't there, neither
> >are a few input devices, such as my (usb) mouse.  
> 
>   Oh? In that case, I would say it's a bug in mdevd-coldplug, which is
> supposed to process everything. Do you have a pattern of stuff that
> isn't processed and should be?

Well, as I said I would always have my audio card & a few input devices
missing. It might be some modules that aren't loaded, and until
then the devices don't show up, hence mdevd-coldplug not finding
them under /sys/dev, but I don't know much else I'm afraid...


>   Wow. That sounds very weird. What in your boot sequence could create
> such a huge burst of events? Can you strace mdevd-netlink to see
> what it's reading from the kernel, and tee its output to check whether
> those are legit events to be sent to mdevd or line noise that is
> improperly filtered at netlink registration time?

The only thing I can think of that would generate a burst of events is
my trigger-uevents script mentionned earlier, but I think I got that
error when it wasn't started on boot...

I'll see if I can strace mdevd-netlink when I get a chance and report
back.


Cheers,


Re: [announce] mdevd-0.0.1.0 - a mdev-compatible uevent manager

2018-01-08 Thread Olivier Brunel
Hey there :)

So I've been using this (instead of your old uevent handler & busybox's
mdev) for a little while now, works great; Thanks!

I do have a few questions though. Originally what I had done was to
start mdevd, run mdevd-coldplug (referred to as coldplug thereafter),
and then start mdevd-netlink (referred to as netlinkd thereafter).

However, somewhat recently I believe I heard (read) you say on IRC that
the recommended way was actually the other way around, so I switched to
start netlinkd first, and then run coldplug.

But, it seems to me that when you're talking about this you expect one
to have two different instances of mdevd, one for netlinkd & another one
for coldplug. That's not how I made things however: mdevd is a longrun
and I simply have both coldplug & netlinkd piped into it.

So for that to work with this new setup/order, I needed to have both do
atomic writes, to ensure things don't get mangled/mixed up. (Wasn't
needed before as I could simply make sure netlinkd only starts after
coldplug, the later being a oneshot.)

Things were good for netlinkd, but not coldplug. So I simply patched it
to flush its output after each uevent. (This being linux and pipe,
I believe atomic writes are ensured so long as we don't go over 4KB,
which I assume won't happen for a single uevent.)

But since that means coldplug now performs a few more write calls, I'm
guessing you might not be interrested in doing such a change upstream,
or am I mistaking?
If not, does that mean the recommended way is really to have two
instances of mdevd? Or is there another/better way I'm not seeing? I
like to have only a single mdevd, one longrun (logged) service, as (I
feel) it should be.


On another topic of sorts: so I start mdevd, netlinkd, and run coldplug.
Cool. But what else would I need to do (upon boot) to ensure everything
has been processed correctly?

By that I mean, currently if I don't do anything else, some things
aren't processed. For instance, my audio card isn't there, neither are
a few input devices, such as my (usb) mouse.

So what I'm doing currently, is have a small trigger-uevents oneshot,
that does this:


#!/usr/bin/execlineb -P
pipeline { find /sys -type f -name uevent -print0 }
forstdin -0 -p f importas -u f f redirfd -w 1 $f echo add


It works, but I'm wondering whether this is the right thing to do, or if
there's a better/more correct way to get there?


Lastly, as a side note, on the doc for netlinkd, regarding the buffer
size, you say "The default is 65536 (which is on the large side)." (Same
as you did before for s6-uevent-listener I believe.)

Now I may be doing something wrong somewhere, but I've always had to use
a larger buffer to make sure things worked correctly myself (That is,
even before w/ s6-uevent-listener).
I'm currently using 1 MB (1048576) to be sure, though it should work
with a smaller one, but I know that even with 131072 (so twice your
"large side" default) I'd always get the following error during boot:

mdevd-netlink: fatal: unable to receive netlink message: No buffer
space available



Thanks!


Re: [PATCH] s6-supervise: Optionally run child in a new pid namespace

2017-07-15 Thread Olivier Brunel
On Sat, 15 Jul 2017 20:24:25 +
"John O'Meara"  wrote:

> You can achieve a PID namespace (and others) using the unshare
> program from util-linux without patching s6. Put the following at the
> top of your run script:
> 
>   unshare -fp --mount-proc
> 
> this also has the advantage of clearly showing which services are in
> their own namespaces when looking at a ps listing, especially for
> forest views ("ps f" or "s6-ps -H")
> 

Though as Jesse explained, this requires some sort of exit/signal
proxing, which isn't the case here. Here the direct child of
s6-supervise remains the daemon itself - in its own pid ns - which is
much better.

As far as showing which services are in their own ns, most namespaces
(i.e. all but pid) won't require a fork and usually you'd get
unshare/nsenter to just exec into the daemon, again to get proper
supervision.

So FWIW I'm very much for this patch myself :) It is a linux-specific
thing, but done as it is w/ a compile-time option this shouldn't be an
issue I'd hope, and it allows simple "proper" supervision of services
running (as pid1) in their own pid ns.

So, thanks Jesse!

Cheers,


Re: [PATCH] Fix openreadnclose failing if errno was nonzero before

2017-05-22 Thread Olivier Brunel
On Mon, 22 May 2017 00:44:28 +
"Laurent Bercot"  wrote:

>   Does this bug manifest in serious ways, for instance making a s6 
> program
> fail when it should not? If it does, I'll need to release a new
> skalibs version just for this >.>

I didn't notice it from a s6 program, but from anopa where e.g.
aa-status would fail since it tries to read e.g. fd-notification
before calling s6_svstatus_read(), so when the file doesn't exist the
later call would fail.

I didn't notice issues w/ s6 programs, but as soon as I fixed this I
recompiled everything (execline, s6, etc) to avoid any issues, so I
didn't look further.


s6-envdir bug/behavior change

2017-05-21 Thread Olivier Brunel
Hey,

Not sure if this was intended or not, but the latest s6-envdir has some
behavior change that at the very least would need to be documented:

Doc says that "s6-envdir reads files in DIR"; And before, if DIR
contained one or more directories they were simply ignored. Now however,
it will fail (111) :
s6-envdir: fatal: unable to envdir: Is a directory

(I think this might be linked to the changes in skalibs, specifically
openreadnclose and error handling.)

Cheers,


[PATCH] Fix openreadnclose failing if errno was nonzero before

2017-05-21 Thread Olivier Brunel
Signed-off-by: Olivier Brunel 
---
 src/libstddjb/openreadnclose.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/libstddjb/openreadnclose.c b/src/libstddjb/openreadnclose.c
index aa61ece..39bb3b9 100644
--- a/src/libstddjb/openreadnclose.c
+++ b/src/libstddjb/openreadnclose.c
@@ -7,6 +7,7 @@
 
 static ssize_t readnclose (int fd, char *s, size_t n)
 {
+  errno = 0 ;
   size_t r = allread(fd, s, n) ;
   if (errno)
   {
-- 
2.13.0



[PATCH] getpid: Fix invalid variable name check

2017-04-12 Thread Olivier Brunel
Test got reverted when converted to skalibs new API (i.e. memchr)
---
 src/execline/getpid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/execline/getpid.c b/src/execline/getpid.c
index 7fb39ac..dd9623a 100644
--- a/src/execline/getpid.c
+++ b/src/execline/getpid.c
@@ -15,7 +15,7 @@ int main (int argc, char const *const *argv, char const 
*const *envp)
   PROG = "getpid" ;
   if (argc < 3) strerr_dieusage(100, USAGE) ;
   len = strlen(argv[1]) ;
-  if (!memchr(argv[1], '=', len))
+  if (memchr(argv[1], '=', len))
 strerr_dief2x(100, "invalid variable name: ", argv[1]) ;
   {
 size_t i = len+1 ;
-- 
2.12.2



Re: [announce] Fall 2016 release

2016-10-28 Thread Olivier Brunel
On Fri, 28 Oct 2016 15:46:25 +
"Laurent Bercot"  wrote:

>   * execline-2.2.0.0
> 
> 
> - Compatibility to skalibs-2.4.0.0.
> - New option "-I" to backtick and withstdinas.
> - New option "-t" to wait: it can now wait for children with a
> timeout.
> - Minor bugfixes.

I believe there was also a "-s" option added to execlineb ;)


Re: How to write an execline helper in execline?

2016-10-16 Thread Olivier Brunel
On Sun, 16 Oct 2016 17:10:23 +
"Laurent Bercot"  wrote:

>   Ew. Shows how much that code is used - in more than two years,
> nobody (including me) has noticed the segfault.
> 
>   Since it's so widely used, and since diving back into environment
> frame pushing/popping gives me headaches, I have half a mind to just
> remove runblock in the next release (which is a major version bump so
> compatibility isn't guaranteed). I'll try fixing the bug for a little 
> more
> time, and if I can't find it, I'll scrap the whole thing.

Had a quick look, and from what i could gather the issue is that in our
case, nothing gets put into v, so by the time pathexec_run() gets
called, v only contains a NULL, and therefore v[0] (well, it's v.s[0]
but hopefully you get what I'm saying) - which is given as first arg -
is indeed, NULL.
So the "file" argument for pathexec_run() is NULL, which ends up in
execvpe() as a call to strchr() with NULL as string/1st arg, and that's
your segfault (in libc, in musl it actually happens in strchrnul).

The attached patch seems to fix it, and also address the other case:

$ ./helper foo bar baz
runblock: fatal: unable to exec bar: No such file or directory

$ ./helper foo bar
runblock: fatal: unable to exec bar: No such file or directory

$ ./helper foo
runblock: fatal: too few arguments

The "return 0" is what made it return w/out even trying to exec into
anything, I'm guessing this was a mistake trying to catch when there
was no rest of command line to exec into?

>   I have implemented your -s suggestion in the latest execline git
> head; that one was easy. Please test it and tell me if it works for
> you.

Yep, works great! Thanks!


Cheers,
>From d01f2fe92be9d50d5592786b5aec08cabb41e001 Mon Sep 17 00:00:00 2001
From: Olivier Brunel 
Date: Sun, 16 Oct 2016 20:38:55 +0200
Subject: [PATCH] runblock: Fix segfault

Signed-off-by: Olivier Brunel 
---
 src/execline/runblock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/execline/runblock.c b/src/execline/runblock.c
index a654023..78a4b59 100644
--- a/src/execline/runblock.c
+++ b/src/execline/runblock.c
@@ -77,7 +77,7 @@ int main (int argc, char const *const *argv, char const *const *envp)
 
   if (flagr)  /* put remainder envvars into v */
   {
-if (++m == sharp) return 0 ;
+if (++m > sharp) strerr_dief1x(100, "too few arguments") ;
 if (!genalloc_ready(char const *, &v, sharp - m + 2))
   strerr_diefu1sys(111, "genalloc_ready") ;
 for (; m <= sharp ; m++)
-- 
2.10.0



Re: How to write an execline helper in execline?

2016-10-16 Thread Olivier Brunel
On Sun, 16 Oct 2016 14:56:14 +
"Laurent Bercot"  wrote:

> >So, please enlighten me if I'm missing a better solution, of course,
> >but in case I'm not, I was wondering how you'd feel about adding a
> >new option to execlineb, e.g. -s, that would require a number as arg
> >(much like -S) but would work as such:  
>   Still considering your -s idea, which should be easy enough to
> implement once I remember how execlineb does things :) but in the
> meantime, wouldn't the "runblock" command help you? Something like
> 
> #!/command/execlineb
> importas FIRSTARG 1
> # work with $FIRSTARG
> runblock -n 1 -r -- 0

Well, that still involves the environment, which I'd like to skip
altogether, but it does seem a bit "easier" since the runblock
call replaces shift+elgetpositionals+emptyenv; However, this gives me
"odd" results.

Consider your script above as ./helper :

$ ./helper foo bar baz
runblock: fatal: unable to exec bar: No such file or directory

Yep, makes sense. Now trying this:

$ ./helper foo bar

Nothing. It ran the helper, then didn't (try to) exec into bar.
Now let's try this:

$ ./helper foo

and it segfaults! (So does "./helper" btw) So maybe "./helper foo
bar" should actually work, but doesn't because of the same bug that
causes segfault?


emptyenv doc bug

2016-10-14 Thread Olivier Brunel
Hey,

Going through some execline doc these days, and I noticed the emptyenv
doc states:

-o : pop environment variables starting with ELGETOPT_. You might want
to do this before executing a final program from a script that uses
elgetpositionals. 

I think this is wrong, as it should be elgetopt instead of
elgetpositionals.

Cheers,


How to write an execline helper in execline?

2016-10-11 Thread Olivier Brunel
Hey,

So I find myself willing to write what I'll call execline helpers -
i.e. prog that do their work, then exec into (the rest of) their command
line, in typical execline fashion - in execline.

IOW an execline script that can be used as execline "command". But the
whole "exec into the (rest of the) command line" appears trickier than
I'd have thought it'd be.

If my helper takes no argument, it's easy enough:

#!/usr/bin/execlineb -S0
# work here
$@

Easy enough, right. (And no need to mess with the environment, which
is always nice.)

Now if it takes one (or more) arguments however, things fall apart. I
doubt I'm the first to want to do such a thing, so I may be missing out
something obvious/clever to do it, but I find myself stuck with this:

#!/usr/bin/execlineb
importas 1 1
# work w/ $1 here
shift -n 1
elgetpositionals
emptyenv -P
$@

That feels like an awful lot of "environment processing" just to get
"proper" substitution: execlineb needs to do its push, shift does the
shift, emptyenv does the pop, and of course there's the whole
importas/elgetpositionals as well. (Not to mention the boilerplate
involed...)

(Also, it's a weird error if there was no "rest of cmdline" to exec
into:
emptyenv: usage: emptyenv [ -p | -c | -o | -P ] prog...
Not the most obvious thing, though not that big a deal either I guess.)

I couldn't find simpler though, unless "cheating" by passing the args
in the environment, then I can do:

#!/usr/bin/execlineb -S0
importas -u ARG ARG
# work w/ $ARG here
$@

Which should work, but makes using the helper much more cumbersome.

So, please enlighten me if I'm missing a better solution, of course,
but in case I'm not, I was wondering how you'd feel about adding a new
option to execlineb, e.g. -s, that would require a number as arg (much
like -S) but would work as such:

- must have at least n+1 args, else error out
- substitute $1 ... $n and $@ in prog, only $@ would start from the
  n+1th arg
- exec in the new argv, w/out touching the environment :)

So back to my helper, I could do:

#!/usr/bin/execlineb -s1
# work w/ $1 here
$@

That's it. It might have other use as well, but at least to write
execline helpers in execline that will take one (or more) args, it
seems to me like a pretty useful thing.

What do you think?


Cheers,


backtick segfault

2016-10-11 Thread Olivier Brunel
Hey,

So first off, quick question about the doc of multisubstitute, where it
reads under Options:
"If a backtick directive was given with the -i option..."

I don't get that. There's no backtick directive listed above, and
trying it use one doesn't seem to work ("unrecognized directive
backtick"). Am I missing something? Is this line not supposed to be
here, or?

Then, speaking of backtick, seems it segfaults if ran w/out a prog to
exec into, e.g:

backtick var { pwd }


Cheers,


[PATCH] bitarray_not: Fix skipping a byte

2016-05-14 Thread Olivier Brunel
Signed-off-by: Olivier Brunel 
---
 src/libstddjb/bitarray_not.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libstddjb/bitarray_not.c b/src/libstddjb/bitarray_not.c
index 4bd95ad..de37b9f 100644
--- a/src/libstddjb/bitarray_not.c
+++ b/src/libstddjb/bitarray_not.c
@@ -12,7 +12,7 @@ void bitarray_not (register unsigned char *s, register 
unsigned int a, register
   {
 register unsigned int i = (a>>3) + 1 ;
 s[a>>3] ^= ~((1 << (a & 7)) - 1) ;
-for (; i < (b>>3) - 1 ; i++) s[i] = ~s[i] ;
+for (; i < (b>>3) ; i++) s[i] = ~s[i] ;
 s[b>>3] ^= (1 << (b & 7)) - 1 ;
   }
 }
-- 
2.8.2



Re: ftrigrd bug

2016-01-07 Thread Olivier Brunel
On Thu, 7 Jan 2016 01:32:01 +0100
Laurent Bercot  wrote:

> On 05/01/2016 15:01, Olivier Brunel wrote:
> > It hasn't been very easy to track down and I'm still unsure of
> > what's causing the issue, many times it seemed like a race
> > condition is at play (or some ordering) since sometimes it would
> > work, other times I only get the EINVAL, and then sometimes it
> > segfaults/lead to EPIPE.  
> 
>   Please try with the latest s6 git and tell me if it fixes the issue.
> 
> (That was stupid: an uninitialized stralloc when you subscribe after
> unsubscribing, so it could crash on realloc or free. But not always,
> so the tests I performed a long time ago didn't catch it.)
> 

Yes, all fixed now; Thanks.


ftrigrd bug

2016-01-05 Thread Olivier Brunel
Hey Laurent,

I don't think I'm doing anything wrong (though if I am, please let me
know) and I'm having some issue with ftrigrd.

What I'm doing is subscribing on some fifodirs, sometimes without and
sometimes with the FTRIGRD_REPEAT flag. For those set, at some point I
unsubscribe. I also add new subscriptions along the way.
I don't think none of that is wrong/unexpected, but on occasion it
doesn't work properly. Specifically, I'll get EINVAL errors from
ftrigrd_update(), or even worse: EPIPE after s6-ftrigrd segfaulted.

It hasn't been very easy to track down and I'm still unsure of what's
causing the issue, many times it seemed like a race condition is at
play (or some ordering) since sometimes it would work, other times I
only get the EINVAL, and then sometimes it segfaults/lead to EPIPE.

As you may have guessed, this happens in anopa when starting services
and waiting for some events. I've tried to make a little test to
reproduce it, but as I said I'm not sure exactly what's the real cause.
It seems to be linked to both unsubscribing & subscribing, which
(sometimes) causes the issue.
E.g. in anopa, if I never unsubscribe it (seems to?) work fine, never
any error.

After many tries, I've come up with something that, at least here in
my latest tries, seems to have s6-ftrigrd segfault every time (I don't
get any EINVAL anymore first, straight to EPIPE, though again I couldn't
say why).

You can find the thing here[1], it's a small C file to compile, and a
folder "sd" which contains 4 servicedirs. To reproduce, you'll need to
start s6-svscan in that "sd" folder, then run the compile test, and
start all services -- I simply use:
for i in {1..4}; do s6-svc -u sd/foo$i; done

With luck, you should get something like this:
---8<--
reading
event=u;id=1
reading
event=u;id=2
reading
event=u;id=3
reading
event=U;id=1
unsubscribing id#1
add some
reading
event=U;id=2
unsubscribing id#2
reading
event=U;id=3
unsubscribing id#3
reading
(none): warning: unable to update: Broken pipe
(none): warning: unable to get event for id#0: Broken pipe
-->8---
And s6-ftrigrd will have segfaulted.

I don't think there's anything wrong in the code, I even took care of
(un)subscribing only after all the ftrigr_{update,check} calls in case
that "messed up" the internals of ftrigr_t somehow, as I became unsure
of whether that was supported or not -- I think so, but if you could
confirm whether or not it actually is I'd appreciate, thanks.

Anyhow, this seems to be reproducable every time for me now, but if not
at first try it a few times, it should happen. Hopefully you can figure
out what's going on & fix it.

I tried to get a backtrace from s6-ftrigrd coredump, but never could get
anything usefull (I should have compiled both skalibs & s6 with
debug symbols in for s6-ftrigrd, but I obviously didn't do it right,
or missed something).

I get either this:
(gdb) bt full
#0  0x00406bff in free ()
No symbol table info available.
#1  0x0100 in ?? ()
No symbol table info available.
#2  0x in ?? ()
No symbol table info available.

Or a slightly longer one, starting with:
#0  0x00407722 in realloc ()


Let me know if there's anything else I can do to help,
-j

[1] http://jjacky.com/ftrigrd.tar.xz


Re: [PATCH] bitarray_clearsetn: Fix possible "overflow"

2016-01-02 Thread Olivier Brunel
On Sat,  2 Jan 2016 17:45:42 +0100
Olivier Brunel  wrote:

> Whenever setting (or clearing) (up to) the last bit in a char, it
> would "overflow" and set/clear all the bits up to it instead.
> 
> Signed-off-by: Olivier Brunel 
> ---
>  src/libstddjb/bitarray_clearsetn.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libstddjb/bitarray_clearsetn.c
> b/src/libstddjb/bitarray_clearsetn.c index b3f46f1..5fee42d 100644
> --- a/src/libstddjb/bitarray_clearsetn.c
> +++ b/src/libstddjb/bitarray_clearsetn.c
> @@ -8,7 +8,7 @@ void bitarray_clearsetn (register unsigned char *s,
> register unsigned int a, reg b += a ;
>if ((a >> 3) == ((b-1) >> 3))
>{
> -register unsigned char mask = ((1 << (a & 7)) - 1) ^ ((1 << (b &
> 7)) - 1) ;
> +register unsigned char mask = ((1 << (a & 7)) - 1) ^ ((1 << ((b
> & 7) ? b & 7 : 8)) - 1) ; if (h) s[a>>3] |= mask ; else s[a>>3] &=
> ~mask ; }
>else
> @@ -18,7 +18,7 @@ void bitarray_clearsetn (register unsigned char *s,
> register unsigned int a, reg if (h) s[a>>3] |= mask ; else s[a>>3] &=
> ~mask ; mask = h ? 0xff : 0x00 ;
>  for (; i < b>>3 ; i++) s[i] = mask ;
> -mask = (1 << (b & 7)) - 1 ;
> +mask = (1 << ((b & 7) ? b & 7 : 8)) - 1 ;

On second thought, this one isn't needed actually. Since we're not
setting only one bit, but up to it, and to get to the last one we'll
have simply used the 0xff mask above, an empty mask (0) works fine
here.
So in this case it doesn't apply, actually. Only the first one, when
setting only the last bit in the char. Commit message should also be
updated to remove the "(up to)"

>  if (h) s[b>>3] |= mask ; else s[b>>3] &= ~mask ;
>}
>  }



[PATCH] bitarray_clearsetn: Fix possible "overflow"

2016-01-02 Thread Olivier Brunel
Whenever setting (or clearing) (up to) the last bit in a char, it would
"overflow" and set/clear all the bits up to it instead.

Signed-off-by: Olivier Brunel 
---
 src/libstddjb/bitarray_clearsetn.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/libstddjb/bitarray_clearsetn.c 
b/src/libstddjb/bitarray_clearsetn.c
index b3f46f1..5fee42d 100644
--- a/src/libstddjb/bitarray_clearsetn.c
+++ b/src/libstddjb/bitarray_clearsetn.c
@@ -8,7 +8,7 @@ void bitarray_clearsetn (register unsigned char *s, register 
unsigned int a, reg
   b += a ;
   if ((a >> 3) == ((b-1) >> 3))
   {
-register unsigned char mask = ((1 << (a & 7)) - 1) ^ ((1 << (b & 7)) - 1) ;
+register unsigned char mask = ((1 << (a & 7)) - 1) ^ ((1 << ((b & 7) ? b & 
7 : 8)) - 1) ;
 if (h) s[a>>3] |= mask ; else s[a>>3] &= ~mask ;
   }
   else
@@ -18,7 +18,7 @@ void bitarray_clearsetn (register unsigned char *s, register 
unsigned int a, reg
 if (h) s[a>>3] |= mask ; else s[a>>3] &= ~mask ;
 mask = h ? 0xff : 0x00 ;
 for (; i < b>>3 ; i++) s[i] = mask ;
-mask = (1 << (b & 7)) - 1 ;
+mask = (1 << ((b & 7) ? b & 7 : 8)) - 1 ;
 if (h) s[b>>3] |= mask ; else s[b>>3] &= ~mask ;
   }
 }
-- 
2.6.4



bitarray: Add first{clear,set}_skip

2015-12-27 Thread Olivier Brunel
Hey Laurent,

So skalibs contains a few functions to work with bit arrays, but only some of
the supported operations can be done with an offset (unless I missed it). One
can obviously clear, set or peek at any bit within the array, but also set/clear
a group of bits from an offset via bitarray_clearsetn().

However, to get the first bit clear/set within a group, the functions
bitarray_first{clear,set}() do not support an offset, even though it might be
useful -- I certainly had the need for it recently.

I think it'd be a nice addition to support this. Since I had to do it, you'll
find below the code for it, feel free not to use it & implement this differently
as you wish ofc (I'm using git to send this as that's better to properly send
code, but this isn't a patch per-se, since I didn't do touch doc/deps or
nothing; Also this would make a terrible commit message :p), or point me to what
I missed to do so already, if that's the case.

For the record, I believe there might be more functions that do not support use
with such an offset, e.g. bitarray_countones(), but I didn't need any of that
so I didn't look further.

Cheers,
-j

---
 src/libstddjb/bitarray_firstclear_skip.c | 22 ++
 src/libstddjb/bitarray_firstset_skip.c   | 22 ++
 2 files changed, 44 insertions(+)
 create mode 100644 src/libstddjb/bitarray_firstclear_skip.c
 create mode 100644 src/libstddjb/bitarray_firstset_skip.c

diff --git a/src/libstddjb/bitarray_firstclear_skip.c 
b/src/libstddjb/bitarray_firstclear_skip.c
new file mode 100644
index 000..1361fb2
--- /dev/null
+++ b/src/libstddjb/bitarray_firstclear_skip.c
@@ -0,0 +1,22 @@
+
+#include 
+
+unsigned int
+bitarray_firstclear_skip (register unsigned char const *s, unsigned int max, 
unsigned int skip)
+{
+unsigned int n = bitarray_div8(max) ;
+register unsigned int i = bitarray_div8(skip) ;
+if (i && s[i - 1] != 0xffU)
+{
+register unsigned int j = skip ;
+skip = i << 3 ;
+if (skip > max) skip = max ;
+while ((j < skip) && bitarray_peek(s, j)) ++j ;
+if (j < skip) return j ;
+}
+for (; i < n ; ++i) if (s[i] != 0xffU) break ;
+if (i == n) return max ;
+i <<= 3 ;
+while ((i < max) && bitarray_peek(s, i)) ++i ;
+return i ;
+}
diff --git a/src/libstddjb/bitarray_firstset_skip.c 
b/src/libstddjb/bitarray_firstset_skip.c
new file mode 100644
index 000..5d77a0c
--- /dev/null
+++ b/src/libstddjb/bitarray_firstset_skip.c
@@ -0,0 +1,22 @@
+
+#include 
+
+unsigned int
+bitarray_firstset_skip (register unsigned char const *s, unsigned int max, 
unsigned int skip)
+{
+unsigned int n = bitarray_div8(max) ;
+register unsigned int i = bitarray_div8(skip) ;
+if (i && s[i - 1])
+{
+register unsigned int j = skip ;
+skip = i << 3 ;
+if (skip > max) skip = max ;
+while ((j < skip) && !bitarray_peek(s, j)) ++j ;
+if (j < skip) return j ;
+}
+for (; i < n ; ++i) if (s[i]) break ;
+if (i == n) return max ;
+i <<= 3 ;
+while ((i < max) && !bitarray_peek(s, i)) ++i ;
+return i ;
+}
-- 
2.6.4



Re: readiness notification from non-subprocess

2015-09-28 Thread Olivier Brunel
On 09/28/15 23:26, Buck Evan wrote:
> When my state machine sees that ./check has succeeded, what will it do?
> I can't use the current notification-fd interface because I'm in an
> unexpected bit of the process tree; this is external program sitting *on
> top* of s6-supervise. It's the parent process.
> 
> I can see that my named-pipe suggestion is flawed. I'm quite willing to
> drop that.
> Can we add a svc - option that tickles the same bit of code?
> 
> Or, can we make it simpler for me to do what s6-supervise does when it gets
> the notification-fd signal?
> It seems to do two things, send a U signal to ./envents/; I can do that.
> It also updates the state in ./supervise/status; that I can't do, without
> writing buggy code.

Just to mention, anopa has a little tool that does just that, I believe:
aa-setready -- See doc[1] and/or source[2]

[1] https://github.com/jjk-jacky/anopa/blob/master/doc/aa-setready.pod
[2] https://github.com/jjk-jacky/anopa/blob/master/src/utils/aa-setready.c

In case that might be of any help,
-j



Re: Bug in ucspilogd v2.2.0.0

2015-08-09 Thread Olivier Brunel
On 08/09/15 10:44, Laurent Bercot wrote:
>  The path leading to the first invocation of readv() hasn't changed,
> but readv() gives different results. My first suspicion is that "logger"
> isn't sending the last character (newline or \0) in the second case
> before exiting, which skagetlnsep() interprets as "I was unable to
> read a full line before EOF happened" and reports as EPIPE.
>  Are you using the same version of "logger" on both machines ?
> 
>  Grrr.  If "logger" starts sending incomplete lines, I may have to change
> the ucspilogd code to accommodate it.

Had a quick look at this (procrastination & stuff :p) and it seems to me
this is probably a bug in logger actually. At some point[1] they started
not to use syslog(3) anymore but implementing things on their own instead.
However, there's a difference with glibc's implementation, specifically
when using a SOCK_STREAM the later adds a NUL byte as record terminator,
which the former does not. Hence there's never a terminating NUL byte
from logger anymore and ucspilogd fails w/ EPIPE.

HTH,
-j

[1]
https://github.com/karelzak/util-linux/commit/1d57503378bdcd838365d625f6d2d0a09da9c29d




Re: [PATCH] s6-uevent-spawner: Fix possibly delaying uevents

2015-06-14 Thread Olivier Brunel
On 06/14/15 21:11, Laurent Bercot wrote:
> On 14/06/2015 14:37, Olivier Brunel wrote:
>> Because of the buffered IO, the possible scenario could occur:
>> - netlink uevents (plural) occur, i.e. data ready on stdin
>> - iopause triggered, handle_stdin() called. The first uevent is
>> processed, child
>>launched, we're waiting for a signal
>> - SIGCHLD occurs, we're back to iopausing on stdin again, only it's
>> not ready
>>yet; Because we've read it all already and still have unprocessed data
>>(uevents) on our own internal buffer (buffer_0)
> 
>  Right, thanks for the catch. I usually avoid that trap, but meh.
>  I committed a simpler change than your patch, please tell me if it fixes
> things for you.

Haven't tested it, but I'm sure it'll "work". In fact I had done
something similar at first, but changed it because I don't think this is
quite correct.

That is, in your test now you're using x[1] even though it might not
have been used in the iopause call before, so while I guess this isn't
random memory, it doesn't really feel right either.

Moreover, imagine this very likely scenario:
- uevent comes in, handle_stdin is called, children forked, and we're
iopausing in x[0] (selfpipe) only.
- SIGCHLD occurs, handle_signals is called, then you test on x[1] which
should still be as it was last time it was actually used, so stating
stdin is readable even though it isn't anymore... and we're gonna block
in handle_stdin.

Not that it's a big deal since at this point we were gonna iopause to
wait for that fd to become readable anyways, since we're not expecting
any signal (as we don't have children anymore). So in effect this works
basically the same, but you're using memory (x[1]) that you probably
shouldn't, and effectively trying to read (and blocking) in handle_stdin
despite having an iopause in place.
You might as well change your test to "if (cont) handle_stdin(...) ;"
then, it'll surely work just as well. But it doesn't feel right, to me
at least, since we have a loop w/ iopause, hence why I went with the bit
longer (and maybe less elegant) version.

-j


[PATCH] s6-uevent-spawner: Fix possibly delaying uevents

2015-06-14 Thread Olivier Brunel
Because of the buffered IO, the possible scenario could occur:
- netlink uevents (plural) occur, i.e. data ready on stdin
- iopause triggered, handle_stdin() called. The first uevent is processed, child
  launched, we're waiting for a signal
- SIGCHLD occurs, we're back to iopausing on stdin again, only it's not ready
  yet; Because we've read it all already and still have unprocessed data
  (uevents) on our own internal buffer (buffer_0)

This could lead to e.g. plug an USB drive, and see only half the uevents be
processed. Generate a new uevent, and see all the "old" uevents then be
processed, and maybe the new ones too, maybe only part of them. Rince, repeat.

We now call handle_stdin() if the buffer isn't empty, so it'll process any such
"buffered" uevents, knowing that it will read if needed (and we can pretty much
assume the data will be there in this case).

Signed-off-by: Olivier Brunel 
---
 src/minutils/s6-uevent-spawner.c | 25 -
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/src/minutils/s6-uevent-spawner.c b/src/minutils/s6-uevent-spawner.c
index 40f088e..f7e2456 100644
--- a/src/minutils/s6-uevent-spawner.c
+++ b/src/minutils/s6-uevent-spawner.c
@@ -219,17 +219,24 @@ int main (int argc, char const *const *argv, char const 
*const *envp)
 
   while (cont || pid)
   {
-register int r = iopause_g(x, 1 + (!pid && cont), &deadline) ;
-if (r < 0) strerr_diefu1sys(111, "iopause") ;
-else if (!r) handle_timeout() ;
+register int ready = !pid && cont ;
+
+if (ready && !buffer_isempty(buffer_0))
+  handle_stdin(&sa, linevar, argv, envp) ;
 else
 {
-  if (x[0].revents & IOPAUSE_EXCEPT)
-strerr_diefu1x(111, "iopause: trouble with selfpipe") ;
-  if (x[0].revents & IOPAUSE_READ)
-handle_signals() ;
-  else if (!pid && cont && (x[1].revents & IOPAUSE_READ))
-handle_stdin(&sa, linevar, argv, envp) ;
+  register int r = iopause_g(x, 1 + ready, &deadline) ;
+  if (r < 0) strerr_diefu1sys(111, "iopause") ;
+  else if (!r) handle_timeout() ;
+  else
+  {
+if (x[0].revents & IOPAUSE_EXCEPT)
+  strerr_diefu1x(111, "iopause: trouble with selfpipe") ;
+if (x[0].revents & IOPAUSE_READ)
+  handle_signals() ;
+else if (ready && (x[1].revents & IOPAUSE_READ))
+  handle_stdin(&sa, linevar, argv, envp) ;
+  }
 }
   }
   if (verbosity >= 2) strerr_warni1x("exiting") ;
-- 
2.4.3



[PATCH] forstdin: Fix possible error/hanging w/ -p

2015-06-09 Thread Olivier Brunel
Basically there's a race condition with signals, and it goes something
like this:
- we launches child1, and add pid1 to the array of pids
- we launches child2, when suddenly!
- SIGCHLD happens: in the handler, we wait() for pid1, removing it from
  pids, then wait for pid2, ignoring it since it's not (yet) in the array,
  wait again and be done
- back in the normal path, we now add pid2 to pids
- later on, we believe there's still a reason to wait (pid2) when in fact,
  there's not. Leading to e.g. ECHILD in waitn()

(Noting that this wasn't limited to a single extra pid in the end.)

Blocking SIGCHLD until we've added the pid to the array ensures this
doesn't happen. (Thus we bring back the call to waitn removed in 5053ea39,
which didn't really fix the issue.)

Signed-off-by: Olivier Brunel 
---
Hey Laurent,

So I have been hit by a bug in forstdin for some time, and finally took some
(time that is) to look into it. Then saw you (tried to) address it recently (in
5053ea39) as I pulled to maybe rebase my fix... However, I'm not sure your fix
really does the trick, plus (as you've seen by now) I have an explanation for
the bug, so there.

This bug would manifest in two ways for me, either forstdin would exit 111 (w/
ECHILD in waitn()), or it would just hang indefinitely. I believe this was two
manifestations of the same issue, and due to some compilation options or
something I'd either get one or the other. I never really looked into it more,
honestly, and the one I faced working on it was the exit 111 (I got the other
from my package compilation, where things are compiled w/ different options
possibly.)

Anyhow, I'm not sure about your fix with sig_pause(), but I did a quick try and
got it to hang indefinitely I'm afraid. So here's a proper fix (I hope), also
restoring the waitn() call.

Cheers,
-j

PS: If I may, little tip/advice re: git: this is why it's better to separate
things when you commit. That is, if you'd made a commit to address this bug
only, one for the bugfix for forbacktickx and another one to prepare for the
rc 2.1.2.2, it might be better/make things easier.
That is, one commit only means/relates to one thing, so when mentioning it it's
clearer what issue we're dealing with. Also it's easier when using blame later
on, or to e.g. revert it, as might have been done here.


 src/execline/forstdin.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/execline/forstdin.c b/src/execline/forstdin.c
index 3fb52eb..b620c89 100644
--- a/src/execline/forstdin.c
+++ b/src/execline/forstdin.c
@@ -128,12 +128,14 @@ int main (int argc, char const **argv, char const *const 
*envp)
   if (!stralloc_0(&modif)) strerr_diefu1sys(111, "stralloc_0") ;
   if (!env_merge(newenv, envlen+2, envp, envlen, modif.s, modif.len))
 strerr_diefu1sys(111, "merge environment") ;
+  if (pids.s) sig_block(SIGCHLD) ;
   pid = el_spawn0(argv[1], argv + 1, newenv) ;
   if (!pid) strerr_diefu2sys(111, "spawn ", argv[1]) ;
   if (pids.s)
   {
 if (!genalloc_append(pid_t, &pids, &pid))
   strerr_diefu1sys(111, "genalloc_append") ;
+sig_unblock(SIGCHLD) ;
   }
   else
   {
@@ -147,11 +149,11 @@ int main (int argc, char const **argv, char const *const 
*envp)
 stralloc_free(&modif) ;
   }
   if (pids.s)
-for (;;)
-{
-  sig_block(SIGCHLD) ;
-  if (!pids.len) break ;
-  sig_pause() ;
-}
+  {
+if (sig_restore(SIGCHLD) < 0)
+  strerr_diefu2sys(111, "restore", " SIGCHLD handler") ;
+if (!waitn(genalloc_s(pid_t, &pids), genalloc_len(pid_t, &pids)))
+  strerr_diefu1sys(111, "waitn") ;
+  }
   return 0 ;
 }
-- 
2.4.2



[PATCH] devd: Fix invalid option used for s6-uevent-listener

2015-04-27 Thread Olivier Brunel
Signed-off-by: Olivier Brunel 
---
 src/minutils/s6-devd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/minutils/s6-devd.c b/src/minutils/s6-devd.c
index 9df5c80..74f2d41 100644
--- a/src/minutils/s6-devd.c
+++ b/src/minutils/s6-devd.c
@@ -63,7 +63,7 @@ int main (int argc, char const *const *argv, char const 
*const *envp)
 }
 if (kbufsz != 65536)
 {
-  newargv[m++] = "-k" ;
+  newargv[m++] = "-b" ;
   newargv[m++] = fmt + pos ;
   pos += uint_fmt(fmt + pos, kbufsz) ;
   fmt[pos++] = 0 ;
-- 
2.3.6



Re: s6-rc design ; comparison with anopa

2015-04-24 Thread Olivier Brunel
On 04/23/15 17:40, Laurent Bercot wrote:
(...)
>  Three kinds of services
>  ---
> 
>  Like anopa, s6-rc works internally with two kinds of services: longrun,
> which is simply defined by a service directory that will be directly
> managed by s6, and oneshot, which is defined by a directory containing
> data (a start script, a stop script, and some optional stuff).
> 
>  s6-rc allows the user to provide a third kind of service: a "bundle".
> A bundle is simply a set of other services. Starting a bundle means
> starting all the services contained in the bundle.
>  A bundle can be used to emulate a SysV runlevel: the user can put all the
> 
> services he needs into a single bundle, then tell s6-rc to change the
> machine
> 
> state to "exactly that bundle".
>  Bundles can of course contain other bundles.
> 
>  A oneshot or a longrun are called atomic services, as opposed to a bundle,
> which is not atomic.
>  Bundles are useful for the user, because "oneshot" and "longrun" are
> often too small a granularity. For instance, the "Samba" service is made
> of two longruns, smbd and nmbd, but it's still a single service. So,
> samba would be a bundle containing smbd and nmbd.

FYI, this is kinda possible with anopa, since one can have "empty"
services, i.e. a servicedir without any script (no run, start nor stop).
Those can be used to order things, but one could also have such a
service (treated as oneshot internally, just without start or stop
script) with a few dependencies.
Then, starting it will start all the dependencies in order, as you'd expect.

However, there isn't anything runlevel-like, as your "switch to that
bundle" command. (One can add "aa=foobar" on the kernel cmdline to use
/etc/anopa/onboot/foobar as listdir to aa-start (during stage 2) instead
of /etc/anopa/onboot/default but that's it).


On that note, one thing you've apparently done/planned is auto-stopping,
whereas there is no such thing in anopa. This is because I always felt
like while auto-starting can be easily predictable/have expected
behavior, things aren't the same when it comes to stopping.

That is, start httpd and it will auto-start dependency php which will
auto-start dependency sqld; fine. But I'm not sure stopping sqld should
stop php, despite the dependency. Maybe one just wants to shut down
sqld, not the whole webserver.

Then there's also the case of, imagine you start foobar & web. web is a
bundle with httpd, php & sqld, foobar is a service w/ a dependency on sqld.
Now you stop web; should sqld be stopped? about foobar? What is the
expected behavior here? I'm not sure there's one, different
people/scenario will have different expectations... it always seemed too
"complicated" so I went with no auto-stopping at all.


>  Also, the smbd daemon itself could want its own logger, smbd-log.
> Correct daemon operation depends on the existence of a logger (a daemon
> cannot start if its logger isn't working). So smbd would actually be a
> bundle of two long-runs, smbd-run (which is the smbd process itself) and
> smbd-log (which is the logger process), and smbd-run would depend on
> smbd-log.

Just so I understand: why are you talking about smbd-log as a separate
service, and not the logger of service smbd, as usual with s6? Or is
that what you're referring to here with smbd-log?

>  Users who want to start Samba don't want to deal with smbd-run, smbd-log,
> nmbd-run and nmbd-log manually, so they would just start "samba", and
> s6-rc would resolve "samba" to the proper set of atomic services.
> 
> 
>  Source, compiled and live
>  -
> 
>  Unlike anopa, s6-rc does not operate directly at run-time on the
> user-provided service definitions. Why ? Because user-provided data is
> error-prone, and boot time is a horrible time for debugging. Also, s6-rc
> uses a complete graph of all services for dependency management, and
> generating that graph at run-time is costly.
> 
>  Instead, s6-rc provides a "s6-rc-compile" utility that takes the
> user-provided service definitions, the "source", and compiles it into
> binary form in a place in the root filesystem, the "compiled".
> 
>  At run-time, s6-rc ignores the source, but reads its data from the
> compiled, which can be on a read-only filesystem. It also needs a
> read-write place to maintain information about its state; this place is
> called the "live". Unlike the compiled, the live is small: it can reside
> in RAM.

I'm not sure where you put the scandir in this? I believe "original"
servicedirs where taken from the source and put into the compiled, and
s6-rc will create the actual servicedirs in the scandir with s6-rc-init,
correct? So that would be part of live?
How about the oneshot scripts?

>  The point of this separation is multifold: efficiency (all checks,
> parsing and graph generation performed at compile-time), safety (the
> compiled can be write-protected), and clarity (separation of user-
> modifiable data, current configuration 

build system license

2015-04-09 Thread Olivier Brunel
Hey Laurent,

Quick question regarding your build system, that is ./configure or
tools/gen-deps.sh and the likes: how are they licensed?

Most of your source files have a "ISC license" mention, but those do
not. Are they also released under the same license, or?

(Asking cause I'd like to use them in a little project of mine, but
wanna make sure it's alright.)

Cheers,
-j


Re: Conditional define in execline

2015-03-20 Thread Olivier Brunel

Not sure if this is what you mean, but maybe something like this:

#!/bin/execlineb -P
backtick -n A { ifte
 { echo 1 }
 { echo 0 }
 test -e ./foobar
}
import -u A
echo ${A}

On 03/20/15 20:21, Gorka Lertxundi wrote:
> Hi,
> 
> Is there an elegant way to achieve a conditional define? something like
> this:
> 
> if (test)
>  A = 0
> else
>  A = 1
> 
> do something with A :-)
> 
> More or less this is what I tried, and it works:
> 
> export fn "\
> ifelse { test }
> {
>   define A 0
>   \\$@
> }
> define A 1
> \\$@"
> 
> execlineb -S0 -c ${fn}
> s6-echo ${A}
> 
> But...I firmly believe that there is a much more elegant way to do this!
> 
> Thanks,
> 



[PATCH] envuidgid: Fix error message

2015-03-20 Thread Olivier Brunel
Signed-off-by: Olivier Brunel 
---
 src/daemontools-extras/s6-envuidgid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/daemontools-extras/s6-envuidgid.c 
b/src/daemontools-extras/s6-envuidgid.c
index 3a58c3c..3712411 100644
--- a/src/daemontools-extras/s6-envuidgid.c
+++ b/src/daemontools-extras/s6-envuidgid.c
@@ -67,7 +67,7 @@ int main (int argc, char const *const *argv, char const 
*const *envp)
 if (n < 0)
   strerr_diefu2sys(111, "get supplementary groups for ", argv[0]) ;
   }
-  else if (insist) strerr_dief2x(1, "unknown user: ", argv[1]) ;
+  else if (insist) strerr_dief2x(1, "unknown user: ", argv[0]) ;
 
   {
 unsigned int pos = 0 ;
-- 
2.3.3



s6-devd, stdin & stdout

2015-03-07 Thread Olivier Brunel
Hi,

I have a question regarding s6-devd: why does it set its stdin & stdout
to /dev/null on start?

This seems non-obvious to me, and isn't documented. I'm specifically
wondering about stdout, since it means all the children/helpers launched
will also have their stdout be /dev/null, so anything they printed
actually gets lost, whereas I was expecting to find it logged, since I
set s6-devd to have its std{out,err} sent to its s6-log.

Unless there's a reason I'm missing, maybe those forced redirections
should be removed?


Also, completely unrelated, but I noticed a "bug" in the doc for
s6-setuidgid: As it turns out, this only calls s6-envuidgid &
s6-applyuidgid, but - according to the doc - there's a difference in
behavior. Specifically, the doc for s6-setuidgid says:
"
If account contains a colon, it is interpreted as uid:gid, else it is
interpreted as a username and looked up by name in the account database.
"
This doesn't seem to be true (anymore?).

Cheers,
-j


[PATCH] examples: Fix syslog LOGSCRIPT

2015-03-03 Thread Olivier Brunel
Log lines are actually prefixed with uids from $IPCREMOTEEUID & $IPCREMOTEEGID,
so they should be acocunted for in the regexs.

Signed-off-by: Olivier Brunel 
---
Also note the need to use \s because, AFAIK, there's no way to use spaces in the
regex then, as space is a delimiter for splitting. This is probably important
enough to be noted alongside in some README in fact, since that's why I've
personally kept them in the run file.

 .../ROOT/img/services-local/syslogd-linux/log/env/LOGSCRIPT| 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/examples/ROOT/img/services-local/syslogd-linux/log/env/LOGSCRIPT 
b/examples/ROOT/img/services-local/syslogd-linux/log/env/LOGSCRIPT
index 65c53e6..e7a1eb7 100644
--- a/examples/ROOT/img/services-local/syslogd-linux/log/env/LOGSCRIPT
+++ b/examples/ROOT/img/services-local/syslogd-linux/log/env/LOGSCRIPT
@@ -1,6 +1,6 @@
-- +^error\. t /var/log/syslogd/error 
-- +^authpriv\. t /var/log/syslogd/auth 
-- +^user\. t /var/log/syslogd/user 
-- +^messages\. t /var/log/syslogd/messages 
-- +^daemon\. t /var/log/syslogd/daemon 
+- +^[0-9]+:\s[0-9]+:\serror\. t /var/log/syslogd/error
+- +^[0-9]+:\s[0-9]+:\sauthpriv\. t /var/log/syslogd/auth
+- +^[0-9]+:\s[0-9]+:\suser\. t /var/log/syslogd/user
+- +^[0-9]+:\s[0-9]+:\smessages\. t /var/log/syslogd/messages
+- +^[0-9]+:\s[0-9]+:\sdaemon\. t /var/log/syslogd/daemon
 f t /var/log/syslogd/misc
-- 
2.3.1



Re: s6 readiness support

2015-02-25 Thread Olivier Brunel
On 02/25/15 23:13, Laurent Bercot wrote:
>  (But really, "s6-ftrig-notify event U" is less hackish than
> "s6-notifywhenup echo".)

Well, except that only the later will also create the "ready" file as
needed (i.e. w/ a TAI timestamp packed in it)...



Re: How to report a death by signal ?

2015-02-18 Thread Olivier Brunel
On 02/18/15 14:20, Laurent Bercot wrote:
> On 18/02/2015 14:04, Olivier Brunel wrote:
>> I don't follow, what's wrong with using a fd?
> 
>  It needs a convention between G and P. And I can't do that, because
> G and P are not necessarily both execline commands. They are normal
> Unix programs, and the whole point of execline is to have commands
> that work transparently in any environment, with only the Unix argv
> and envp as conventions.

But isn't the whole anything >= 128 will be reported as 128, and
anything higher is actually 128+signum also a convention that both needs
to agree upon?

P will "do something" to report info about C to G, and P and G needs to
agree about how said info will be reported. Using 128+signum is one way,
using an fd for the full/correct info is another.

The later being an option, it wouldn't change what P returns, but be an
additional means to provide accurate information to grandprocess G
should they need to.
Or just, like shells, assume it's not needed and simply only do the
128+signum convention.

Noting that shells do not actually clamp the exit code to 128. As
illustrated by Peter's example, shells return the exit code (up to 255
included), or 128+signum.
So assuming no signal, you get the accurate exit code. But of course,
with anything higher than 128 there's no way of knowing if it was an
exit code or a signal (unless you know exit codes don't go that high).
(Clamping provides better results though, so I'm not saying don't do it;
Just the difference shall probably be pointed out/documented.)


>> Cause that was my idea as well: return the exit code or 255.
> 
>  I was considering it for a while, then figured that the signal number
> is an interesting information to have, if G remotely cares about
> C crashing. I prefer to reserve the whole range of 128+ for
> "something went very wrong, most likely a crash at some point, and
> if you get 129+, it was directly below you and you get the signal
> number".
> 
> 
>> Though if you want "shell compatibility" you could also have an option
>> to return exit code, or 128+signum when signaled, and similarly one
>> would either be fine with that, or have to use the fd for full/complete
>> info.
> 
>  Programs that can establish a convention between one another are easy
> to deal with. If I remember to document the convention (finish scripts
> *whistle*)
> 



Re: How to report a death by signal ?

2015-02-18 Thread Olivier Brunel
On 02/18/15 13:27, Laurent Bercot wrote:
> On 18/02/2015 11:58, Peter Pentchev wrote:
>> OK, so the "not using the whole range of valid exit codes" point rules
>> out my obvious reply - "do what the shell does - exit 128 + signum".
> 
>  Well the shell is happily ignoring the problem, but it doesn't mean
> it has solved it. The shell reserves a few exit codes, then does some
> best effort, hoping its invoked commands do not step on its feet.
> It works because most commands will avoid exiting something > 125,
> but it's still a convention, and most importantly, the shell itself
> does not follow that convention (it obviously cannot!)
>  So, something like sh -c "sh -c foobar" does not report errors
> properly: for 126 and 127, there's no way to know if the code belongs
> to the inner shell or the outer shell, and for 128+, there's no way
> to know if the inner shell or the foobar process got killed.
> 
>  Shells get away with it because when they're nested, it's usually
> auto-subshell magic and users don't want to know about the inner
> shell; but here, I'm trying to solve the problem for execline commands,
> and those tend to be nested a lot - so I definitely cannot reserve codes
> for the outer command, because the inner command may very well use the
> same ones too.
> 
> 
>> Now the question is, do you want to solve this problem in general, or do
>> you want to solve it for a particular combination of programs, even if
>> new programs may be added to that combination in the future, but only
>> under certain rules?  If it's the former (in general), then, sorry, I
>> don't have a satisfactory answer for you, and the fact that the POSIX
>> shell still keeps the "exit 128 + signum" behavior mostly means that
>> nobody else has come up with a better one, either (or it might be
>> available at least as some kind of an option).
> 
>  It just means that nobody cares about shell exit codes. Error handling,
> if any, is done inside of shell scripts anyway; and in most scripts, a
> random signal killing a running command isn't even something people think
> about, and I'm sure there are hilarious behaviours hiding in dark corners
> of very popular shell scripts, that fortunately remain asleep to this day.
> 
>  For execline, however, I cannot use the same casual approach. Execline
> scripts live and die by proper exit code reporting, and carelessness may
> lead to very obvious breakage.
> 
> 
>> Personally, I quite like the idea of some kind of a pipe (be it a
>> pipe(2) pair of file descriptors or an AF_UNIX/PF_UNSPEC socketpair or
>> some other kind of communication channel based on file descriptors),
>> even if it is only unidirectional:
> 
>  Oh, don't get me wrong, I'm a fan of child-to-parent communication via
> pipes, and I use it wherever applicable. Unfortunately, the child may
> be anything here, so I need something generic.

I don't follow, what's wrong with using a fd? Cause that was my idea as
well: return the exit code or 255. If there's a need to differentiate
between exiting 255 or a signal, an option taking a fd shall be used,
and the process P will then also write to said fd the exit code, or 255
+ signal number (as a string).

Note the the child C has nothing to do with this here, it is P that
waits for it and gets the wstat, possibly writing to the fd given as
option, and it is grandparent G that needs to specify an fd to P and
then read it to get the full status info.

Though if you want "shell compatibility" you could also have an option
to return exit code, or 128+signum when signaled, and similarly one
would either be fine with that, or have to use the fd for full/complete
info.


>  Thanks for your input !
> 



Re: loopwhilex

2015-02-18 Thread Olivier Brunel
On 02/18/15 01:17, Laurent Bercot wrote:
> On 18/02/2015 00:22, Olivier Brunel wrote:
>> - First off, I found the doc to be a bit confusing, because you
>> alternate between talking about when loopwhilex keeps running prog, and
>> when it stops/exits.
> 
>  Yes, it's confusing, but it's still accurate.
> 
> 
>> In fact, it's confusing enough that you yourself got it wrong :p
> 
>  Nope, the doc is correct here. At least in intent; if loopwhilex's
> behaviour is different, then it's a bug.

Alright, so let me quote you from that very mail, about behavior when
using both -n and -x :

>  With the -x option, the "exit loop" codes are the listed argument
> and the "continue looping" code is all the rest.

With -x, breakcodes are when loopwhilex exits. So with -n, they are when
it continues, since it negates it, correct?
Let me repeat: you're saying with both -n & -x, breakcodes are when it
*does* loop.


Ok, now let me quote you the doc for the same case, using both -n & -x :

It will "run prog... as long as it exits a code that is not listed in
breakcodes" (there's even emphasis on the "not").
Let me repeat: the doc is saying with both -n & -x, breakcodes are when
it does *not* loop.

Clearly, one of you is wrong. I still say the doc is wrong there.



>  The problem is, loopwhilex behaves differently when the -x option
> is given and when it's not.
>  Without the -x option, the "continue looping" code is 0 and the
> "exit loop" code is all the rest.
>  With the -x option, the "exit loop" codes are the listed argument
> and the "continue looping" code is all the rest.
>  That is why it's confusing: the "explicit set of codes" vs.
> "all the unlisted codes" meaning is reversed when you give the -x
> option.
> 
>  The -x option was added as an afterthought, and the -n option
> after that one again. I only later realized how confusing it was.
> The right solution is to replace the -x option with another one, where
> you list the "ok" codes, not the "exit" ones, and if you want "-x"
> behaviour, you add "-n". I'll do that for the next release.
> 
> 
>> - The phrasing on the doc led me to think that if prog is killed by a
>> signal, loopwhilex exits (that signal number). But looking at the code I
>> see that that's not the case, instead it will treat it as if prog had
>> returned 126. Why that magic number? Also, shouldn't it always exit when
>> prog was killed by a signal?
> 
>  Hmmm, it probably should. I'll check and fix it accordingly.
>  The magic number comes from a time when I didn't have a clear vision
> of what to do when programs get killed, and I didn't want to think
> about it too much, so I treated a death by signal like an exit with
> failure by default.
> 
>> I was gonna say I'd like to get the exit code from prog that caused the
>> loop to end. So, I was thinking an option to make loopwhilex return the
>> exit code from prog, and maybe be killed by the same signal if prog was
>> signaled.
> 
>  Signal propagation is dangerous, and I don't like to use it when it's
> not strictly necessary.
> 
> 
>>loopuntilx [ -n ] [ -x breakcodes ] { progloop... } prog...
> 
>  That was actually how it worked in early versions of execline; the
> command was called "loopwhile". But with use, I realized that
> implementing a sequence of operations where it's not necessary led to
> code duplication; add a "foreground" around the "loopwhilex" if you
> want that behaviour.

Yes, except when you want the exit code from progloop of course, which
was what I was trying to address here in the first place...

>  I like chain loading, a lot, but sometimes exiting is just the right
> thing. :) That's why there's forx (replaced for), forbacktickx (replaced
> forbacktick), and ifte (replaced ifthenelse, that is still around because
> the order of blocks is more intuitive and I actually still have a script
> somewhere relying on ifthenelse -s *shudder*, but that should go the way
> of the dodo at some point).
> 
>  But yes, even with a foreground construct, your following comment is
> valid:
> 
>> (As a side note though, if foreground would set that ? environment
>> variable to 256 instead of 111 when signaled it might be better, since
>> 111 is a valid exit code after all.)
> 
>  Absolutely, it's the right thing, like the fix to finish scripts.
> 



loopwhilex

2015-02-17 Thread Olivier Brunel
Hi,

Couple of things regarding loopwhilex:

- First off, I found the doc to be a bit confusing, because you
alternate between talking about when loopwhilex keeps running prog, and
when it stops/exits.

In fact, it's confusing enough that you yourself got it wrong :p
"
-n : negate the test: run prog... as long as it exits non-zero (or exits
a code that is *not* listed in breakcodes).
"

See that "not" that's in italic on the HTML page, well I believe it
should not actually be there, as -n actually means to run prog... as
long as it exits a code that *is* listed in breakcodes.

Again, might feel odd/confusing because the line above also mentions
something when it *is* listed in breakcodes, but it speaks about exiting
the loop instead of ending it there.

I feel picking one and only talking about that one, to make things
clearer/easier to follow, could be a good idea.

I'd suggest going with the "exits when"; say loopwhilex runs prog until
it exits non-zero, or exits one of the breakcodes. Then -n means to end
the loop if it exits zero, or not listed in breakcodes.

Even makes it easier to read: "-x 2" means stop on 2; "-n -x 0" means
stop on non-zero. It might feel a bit odd because "loopwhilex -x 2"
doesn't mean while it returns 2, but while it doesn't, but that's how it
works I guess (loopuntilx might have been a "better" name?).


- The phrasing on the doc led me to think that if prog is killed by a
signal, loopwhilex exits (that signal number). But looking at the code I
see that that's not the case, instead it will treat it as if prog had
returned 126. Why that magic number? Also, shouldn't it always exit when
prog was killed by a signal?


- And then, I was gonna ask for something, but as I kept thinking about
things I went another way, and in the end won't need/use loopwhilex
after all. But let me just mention the idea anyhow, just in case:

I was gonna say I'd like to get the exit code from prog that caused the
loop to end. So, I was thinking an option to make loopwhilex return the
exit code from prog, and maybe be killed by the same signal if prog was
signaled.

But then I thought as an alternative/better choice, how about a new
loopuntilx that works the same as loopwhilex, but that takes another
block as input:
  loopuntilx [ -n ] [ -x breakcodes ] { progloop... } prog...

It would run progloop until it exits non-zero (or one of the breakcodes)
(unless -n ofc) and then exec into prog, having set the ? environment
variable to the (last) exit code of progloop, or 256 if signaled.

But again, I won't need this after all, so I'm fine if you don't wanna
add this.


(As a side note though, if foreground would set that ? environment
variable to 256 instead of 111 when signaled it might be better, since
111 is a valid exit code after all.)

Cheers,
-j


Re: new s6-log

2015-02-09 Thread Olivier Brunel
On 02/09/15 00:06, Laurent Bercot wrote:
> On 08/02/2015 22:12, Olivier Brunel wrote:
>> Except for that bit. I don't like that, and I'd really like an option to
>> turn that behavior off. Specifically, in my case the only scenario I can
>> imagine where the write would fail, i.e. daemon is down, would be
>> because it crashed/was restarted; Neither of which should mean to stop
>> the whole process.
> 
>  It's a question of safety. Imagine the daemon dies and never comes
> back up. Should I let lines accumulate indefinitely in s6-log's bufalloc_1,
> eating more and more memory until the inevitable oom ? Definitely not.
> Then what ? Any other solution loses log lines.
> 
>  Plus, a write error likely means that the pipe between s6-log and the
> auxiliary logger is broken. It can never be used again. It's impossible
> to resume writing to another instance of the auxiliary daemon without
> restarting s6-log. So, why keep stuff in a buffer that can probably never

Well, I may have mixed up memories of the idea of s6-log using a file vs
simply stdout, as in with a file it could simply (re)open it, no need to
restart then actually (and since I'm actually gonna use a fifo, that'd
have worked). Anyhows...

> be flushed again ? If you need that auxiliary logger, you have to restart
> the pipeline. Kill your entire logging service, which will recreate the
> pipe.
> 
>  You can avoid killing s6-log by fd-holding the reading end of the pipe:
> have some other process keep it open. That's easy to do if you're
> supervising the auxiliary logger. And in that case, s6-log will never
> receive an EPIPE, it will just detect non-readability, and keep the
> log lines in memory until your auxiliary logger has restarted. That is
> the way to obtain the behaviour you want.

Yes, I think I spoke too quick and was silly/wrong as a result. But
you're right, and this will work fine; I just played a bit with the
fdholding tools, seems to work very nicely/easily. :)

Quick question though, in case I missed something obvious: what did you
have in mind when you said: "[keeping the fd open is] easy to do if
you're supervising the auxiliary logger" ?

In my case I'm using a fifo (between s6-logs & the auxiliary logger), so
I just open it and store it to a fdholderd prior, and the auxiliary
logger retrieves it on start. Is there another/a better way to do this?
(because I'm not sure how the fact that it's supervised helps/changes
anything?)
Or were you thinking of another situation/configuration?



Re: new s6-log

2015-02-08 Thread Olivier Brunel
On 02/07/15 17:17, Laurent Bercot wrote:
> On 27/01/2015 01:10, Laurent Bercot wrote:
>>   Something like that. I haven't given it any thought yet
> 
>  So I've experimented a lot with that. I've put in knobs
> and turned and tweaked them, I've put guards on the number of
> forks per second, and guards on the last time since a fork
> happened, and I've made it possible to fork several handlers
> at a time or only one handler at a time, etc. etc.
> 
>  Nothing is satisfying. The overwhelming conclusion to my
> experiments was: it's doable, but safety is in direct opposition
> to reliability here - the functionality has conflicting goals.
> Either the user risks forking a zerg swarm, or he risks missing
> log lines. It's possible to still get notification of a missed
> log line even without the content, but it's not possible to predict
> what content will be missing.
>  Unix was clearly not made for this. You definitely can't safely
> and reliably make a control flow depend on an arbitrary data flow.
> 
>  So, I did what I usually do in those cases: push the problem onto
> the users. :P
>  I went back to the "log to a file" approach, but of course I didn't
> make s6-log write to files. I just added a "log to stdout" directive.
> 
>  So if you want to be notified when a certain line arrives, you just
> start s6-log by pipelining it into a long-lived program that notifies
> you when it gets a line; the s6-log instance should have a script
> that, among other things, selects the interesting line and sends it to
> stdout.
>  (If s6-log is supervised, when you pipeline, make sure s6-log gets
> the run script's pid.)

Just did some quick tests, and it seems I'll be able to do what I need
just fine with this, yes...

>  That's one long-lived program that reads lines and does stuff, instead
> of a flurry of short-lived programs. I feel a lot more comfortable with
> that. The program can even die when it's not interested anymore, s6-log
> will just stop storing stuff to send to stdout after a failed write.

Except for that bit. I don't like that, and I'd really like an option to
turn that behavior off. Specifically, in my case the only scenario I can
imagine where the write would fail, i.e. daemon is down, would be
because it crashed/was restarted; Neither of which should mean to stop
the whole process.
I can understand that because it crashed/was restarting some messages
coming in at that exact time be lost/not processed, but that certainly
shouldn't mean no other messages should be sent ever again! (At least,
not in my case, so as a default why not, as long as I can turn it off.)

>  I've used the opportunity to overhaul s6-log and make some changes.
> Notably, there can now be ISO 8601 timestamps - for the log contents,
> not for the archive filenames. Less notably, but artistically nice:
> the script now lives in the stack, not in the heap.

Little bug in the refactoring though it seems, tainstamps aren't valid
no more; See patch.

>  Available on the current s6 git head. (Needs the current skalibs git
> head.) Please tell me if it works for you.
> 



[PATCH] s6-log: Fix tainstamp being truncated

2015-02-08 Thread Olivier Brunel
Signed-off-by: Olivier Brunel 
---
TIMESTAMP is 25 char long, so just enough for the tainstamp with its @-prefix.
This was actually overwriting the last character in the tainstamp!

 src/daemontools-extras/s6-log.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/daemontools-extras/s6-log.c b/src/daemontools-extras/s6-log.c
index d460a90..8a5afc0 100644
--- a/src/daemontools-extras/s6-log.c
+++ b/src/daemontools-extras/s6-log.c
@@ -912,11 +912,11 @@ static void script_run (scriptelem_t const *script, 
unsigned int scriptlen, char
   int flagselected = 1, flagacted = 0 ;
   unsigned int i = 0, hlen = 0 ;
   char hstamp[32] ;
-  char tstamp[TIMESTAMP] ;
+  char tstamp[TIMESTAMP+1] ;
   if (gflags & 1)
   {
 timestamp_g(tstamp) ;
-tstamp[TIMESTAMP-1] = ' ' ;
+tstamp[TIMESTAMP] = ' ' ;
   }
   if (gflags & 2)
   {
@@ -953,7 +953,7 @@ static void script_run (scriptelem_t const *script, 
unsigned int scriptlen, char
   for (j = 0 ; j < script[i].actlen ; j++)
   {
 act_t const *act = script[i].acts + j ;
-siovec_t v[4] = { { .s = tstamp, .len = act->flags & 1 ? TIMESTAMP : 0 
}, { .s = hstamp, .len = act->flags & 2 ? hlen : 0 }, { .s = (char *)s, .len = 
len }, { .s = "\n", .len = 1 } } ;
+siovec_t v[4] = { { .s = tstamp, .len = act->flags & 1 ? TIMESTAMP+1 : 
0 }, { .s = hstamp, .len = act->flags & 2 ? hlen : 0 }, { .s = (char *)s, .len 
= len }, { .s = "\n", .len = 1 } } ;
 switch (act->type)
 {
   case ACTTYPE_FD1 :
-- 
2.3.0



ifthenelse bugged?

2015-01-31 Thread Olivier Brunel
Hi,

So I may be doing/understanding something wrong, but it seems to me that
ifthenelse isn't exactly working as expected:

% execlineb -c 'ifthenelse { exit } { echo ok } { echo fail } echo next'
ok
ok
next
next

According to the doc, it should be the same as the following:

% execlineb -c 'foreground { ifte { echo ok } { echo fail } exit } echo
next'
ok
next

Obviously it isn't, as only the later works as expected (or, as I do
expect). A quick look at the code for ifthenelse has me wondering why
there's a fork() at line 44:

  argv[argc1] = 0 ;
  pid = fork() ;
  pid = el_spawn0(argv[0], argv, envp) ;
  if (!pid) strerr_diefu2sys(111, "spawn ", argv[0]) ;

Is that really normal? And if so, where's my error using ifthenelse?

Cheers,
-j


Re: Feature requests for execline & s6

2015-01-26 Thread Olivier Brunel
On 01/27/15 00:12, Laurent Bercot wrote:
> On 26/01/2015 23:52, Olivier Brunel wrote:
>> That looks good, except that I need a default value, in case the file
>> doesn't exist. If there's a way to work a default value in that case
>> then that'd be fine, yes.
>> ...is there a way to add a default value if FILE doesn't exist?
> 
>  I will add a "-D default" option to backtick, for when the subprocess
> exits nonzero. I've wanted this feature for some time, but only now
> realized that I wanted it. Thanks for helping me clarify my mind. :)

My pleasure :)


>> (re: misconfiguration, in what way though? It could lead to the program
>> being triggered for each new line, but that doesn't mean uncontrollably
>> spawning children?
> 
>  Well, some services log a lot. Spawning a process for every new line
> can be really, really heavy. (And I don't even want to imagine how heavy
> it would be on Solaris.)
>  I really don't mind forking and execing, as can be proven by execline;
> but potentially forking every time a daemon outputs a newline is still
> really scary. I need to find the right safety guards before I implement
> that.

I see... so you're thinking e.g. a control directive to set the maximum
amount of time the action can be triggered per minute or something?


>> It might be a lot, but with a rotation every 4096 bytes one might
>> already have frequent calls to the processor, no?)
> 
>  Sure, with s4096, but who limits their log files to 4k ? It requires
> specific configuration to do that; the admin really wanted it to
> happen, it's controlled. A daemon's output matching a given regexp
> - which could be .* - is a lot less predictable. And if the daemon is
> spewing nothing but newlines, you could find yourself forking 4096 times
> more than you're already doing with the processor...
> 



Re: Feature requests for execline & s6

2015-01-26 Thread Olivier Brunel
On 01/26/15 23:18, Laurent Bercot wrote:
> 
>> - execline: I'd like the addition of a new command, e.g. readvar, that
>> would allow to read the content/first line of a file into a variable.
>> IOW, something similar to importas (including an optional default
>> value), only instead of specifying an environment variable one would
>> give a file name to load the value from (in a similar manner as to what
>> s6-envdir does, only for one var/file and w/out actually using
>> environment variable).
> 
>  How about
> "backtick -n SOMETHING { redirfd -r 0 FILE cat } import -U SOMETHING" ?
> 
>  Sure it's longer, but it's exactly what the "readvar" command would do.
> You can even program the "readvar" command in execline. ;)

That looks good, except that I need a default value, in case the file
doesn't exist. If there's a way to work a default value in that case
then that'd be fine, yes.
...is there a way to add a default value if FILE doesn't exist?


>  If for some reason it's inconvenient for you, I can envision writing
> something similar to readvar, but it *is* going to use the environment:
> I've been trying to reduce the number of commands that perform
> substitution themselves, so that passing state via the environment is
> prioritized over rewriting the argv, and "import" is kind of the
> one-stop-shop when you need substitution. I don't want to go back on
> that intent and add another substituting command. Is it a problem for
> you to use the environment ?

No it's not, it just felt unneeded when thinking of readvar, is all. But
i could combine it with import, as I said what I really want is to have
a default value if the file doesn't exist, instead of nothing/empty
string, skipping/not using the environment was/is not a requirement at all.


>> - s6-log: I think a new action to write to a file could be useful.
> 
>  The problem with that is the whole design of s6-log revolves around
> controlling the size of your logging space: no uncontrolled file
> growth. Writing to a file would forfeit that.
> 
>  What I'm considering, though, is to add a "spawn a given program"
> action, like a !processor but every time a line is selected and triggers
> that action. It would answer your need, and it would also answer
> Patrick's need without fswatch. However, it would be dangerous: a
> misconfiguration could uncontrollably spawn children. I'll do that when
> I find a safe way to proceed.

Yeah that would work for me... I thought the "writing to file" action
was maybe more "generic," but I get your point re: controlling the
logging size.

Running an external program would work for me just as well, yes. Looking
forward to this nice little addition.

(re: misconfiguration, in what way though? It could lead to the program
being triggered for each new line, but that doesn't mean uncontrollably
spawning children?
It might be a lot, but with a rotation every 4096 bytes one might
already have frequent calls to the processor, no?)

-j



Feature requests for execline & s6

2015-01-26 Thread Olivier Brunel
Hi Laurent,

A couple things I'd like to ask, on different tools :

- execline: I'd like the addition of a new command, e.g. readvar, that
would allow to read the content/first line of a file into a variable.
IOW, something similar to importas (including an optional default
value), only instead of specifying an environment variable one would
give a file name to load the value from (in a similar manner as to what
s6-envdir does, only for one var/file and w/out actually using
environment variable).
This would be pretty useful for e.g. using options in scripts.

- s6-log: I think a new action to write to a file could be useful.
Specifically, what I need done is have some actions occur on certain log
events. I'm thinking that if the logging script in s6-log could write
the selected lines to a file, said file could actually be a FIFO where a
supervised app would be listenning/processing what's written there as
needed.
This seems to me like it'd be a good solution, but s6-log doesn't
provide a way to do that, unless I'm missing something. I can't use
status file because they're done atomically, i.e. a new file is written
and renamed, and I don't want the whole logdir mechanics of rotation,
etc Just simply writing lines to a file.
Unless you recommend a better way to accomplish what I'm trying to do?

- Minor thing, but just to make sure I didn't misunderstand something
really: in s6_svstatus_{read,write} you have code like the following:

char tmp[n + 2 + sizeof(S6_SVSTATUS_FILENAME)] ;
byte_copy(tmp, n, dir) ;
byte_copy(tmp + n, 2 + sizeof(S6_SVSTATUS_FILENAME), "/"
S6_SVSTATUS_FILENAME) ;

Now, shouldn't thoose 2 simply be 1-s, since the NUL is already
accounted for with sizeof? Or am I missing/misunderstanding something here?

Regards,
-j


Re: s6 service 'really up' clarification

2015-01-13 Thread Olivier Brunel
On 01/13/15 02:46, Laurent Bercot wrote:
> On 13/01/2015 01:35, Olivier Brunel wrote:
>> My idea was that it would be made aware of it, basically adding a new
>> 'command' to its fifo, so what s6-notifywhenup would do is write 'U'
>> there, and supervise would then update its state and be the one emitting
>> the event. (notifywhenup taking not a fifodir but a servicedir then.)
>>
>> And for cases where one doesn't use notifywhenup, e.g. because there's
>> another way for the daemon to notify it's ready, then s6-svc could have
>> a -U to do what notifywhenup does: tell supervise to update the state &
>> emit the event U.
> 
>  No, I cannot do that. We can't have other processes messing with the
> readiness state, and adding -U to the s6-svc interface is going to
> accomplish exactly that. Users are wonderfully creative when it comes
> to using tools for a totally different purpose than what they were made
> for, and I want to make sure this does not happen.
>  s6-svc is a tool to control the service, not to make s6-supervise store
> user-provided information. If we need to store user-provided information,
> it should be done another way.

Yeah I agree, and I originally didn't like this for the same reason.
However, I figured if supervise supports it when written to its control,
it might as well be in svc, if only for consistency, plus it would be
easier for cases where notifywhenup cannot be used and another mechanism
indicates the ready state.

However, whether or not this is supported in svc, I still think it is
the right thing to do: have supervise maintain that info in the
statusfile, and so all other s6-* tool be aware of that state properly.

Even though it might not come from the process/service directly, I feel
it still is service information, not user-provided information (even
though it is. But one could argue that something like a file "down" also
is, yet supervise uses it).


>> This might not always be useful/used, but it's there when needed and it
>> feels to me like the correct way to have this information (as opposed to
>> have e.g. other tool maintaining a "parallel" state on their own...).
> 
>  Well I maintain that this information is not relevant to s6-supervise,
> that only knows about up and down. s6-supervise can tell when the service
> becomes not ready, but it cannot tell when it becomes ready - that event
> can only be provided via the daemon itself, so another tool is needed
> anyway. I'm very reluctant to pass the information back to s6-supervise
> if it can be stored directly by this other tool.
> 
>  I'll probably just have s6-supervise unlink supervise/ready on event
> up or down, and have s6-notifywhenup create supervise/ready on readiness

Yeah but then either s6-* tools still don't actually known anything
about that ready state now, or they do simply by using another file
instead of the statusfile, which really is all that should be needed.

Besides, you say you don't want supervise to know about this, yet it
still has to maintain it. If it needs to remove the ready file whenever
the service goes down, it effectively means it is in charge of
maintaining that information, or only half in charge. The other half
being left to some other tool.
And now we can easily have tools thinking a service is ready because
there's a ready file, even though the even U was never emitted...
whereas if supervise handles it fully, not only removing but also
creating the ready file (or, then a flag in the statusfile) & emitting
the event, it's much simpler to get consistent behavior: it emits U when
updating the statusfile to ready, emits d when updating it to down. No
one simply creates/unlinks a file without a corresponding event, better
consistency.

> notification, so s6-notifywhenup needs to have write access to
> supervise/ but not to supervise/status. And for other tools, it's easy
> enough to touch a file.

Note that I never expected notifywhenup to write to the statusfile, but
the control fifo of supervise, the later doing the status update &
event. supervise/status would be written to only by supervise.



Re: s6 service 'really up' clarification

2015-01-12 Thread Olivier Brunel
On 01/13/15 00:38, Laurent Bercot wrote:
> On 12/01/2015 23:27, Patrick Mahoney wrote:
> 
>> So, speculating, it seems that 's6-svwait -U' on a service that is
>> currently 'down' either waits forever, or returns when something sends a
>> 'U' event. But when the service is 'up' (but no U event ever happened),
>> 's6-svwait -U' returns immediately as if the service was 'really up'.
> 
>  Yes, and I agree that is counter-intuitive and problematic, but I
> haven't come up with a good solution yet.
> 
>  The issue is that s6-supervise stores the service state in
> supervise/status
> and has no notion of service readiness. For s6-supervise, either the
> service
> is up (pid nonzero) or it is down (pid zero), it cannot know whether the
> service is "really up" or not.
>  And adding an understanding of service readiness to s6-supervise would:
>  - make no sense: it's a process supervisor, no matter what the process
> does.
> I use it for cron-like jobs (do the job then sleep for a while), for
> instance,
> where there's no notion of service readiness.
>  - be absolutely bloated and ugly, since there should be a feedback
> canal from
> the service to s6-supervise, and I really don't want to go there.

If I may... this is actually something I have been thinking about, and
was planning on suggesting: that supervise should be aware of that 'U'
state.

My idea was that it would be made aware of it, basically adding a new
'command' to its fifo, so what s6-notifywhenup would do is write 'U'
there, and supervise would then update its state and be the one emitting
the event. (notifywhenup taking not a fifodir but a servicedir then.)

And for cases where one doesn't use notifywhenup, e.g. because there's
another way for the daemon to notify it's ready, then s6-svc could have
a -U to do what notifywhenup does: tell supervise to update the state &
emit the event U.

That way s6-svwait or s6-svstat could also be aware of the U state
properly, since it would in fact be in the statusfile, so there's always
a proper/consistent state known to all s6-* tool.

This might not always be useful/used, but it's there when needed and it
feels to me like the correct way to have this information (as opposed to
have e.g. other tool maintaining a "parallel" state on their own...).


>  s6-svwait listens on the event/ fifodir then checks on supervise/status to
> know the initial state of the service before any notification arrives. But
> that initial state is only "down" or "up", as s6-supervise wrote it;
> it's never
> "ready". So for now, s6-svwait -U assumes that "up" means "ready". Which
> causes the behaviour you observed.
> 
>  To solve that, the up vs. ready state should be stored in another file,
> that
> s6-svwait -U should check. The logical place to do that operation is in
> s6-notifywhenup.But that raises a few other issues:
> 
>  - That would more or less enforce s6-notifywhenup as *the* readiness
> notification tool to use with s6-svwait. If daemons want to use another
> notification mechanism than the one supported by s6-notifywhenup, there
> will
> need to be another compatibility tool to convert notification mechanisms.
> This can become pretty ugly really fast.
> 
>  - That means s6-notifywhenup has to have write permissions to supervise/
> instead of simply read permissions. So, basically, s6-notifywhenup has to
> run as root. It's nothing complex, but it's still more root code that the
> admin has to trust.
> 
>  - s6-notifywhenup isn't around when the daemon dies, so it cannot update
> the new state file. Only s6-supervise is around, so it has to do the job.
> That is an ad-hoc hook that needs to be added to s6-supervise. Again,
> nothing complex, but by definition, ad-hoc is ugly.
> 
>  All this is probably still better than the current behaviour, so I may add
> it  in a future release.
> 



Re: building (with) execline

2015-01-12 Thread Olivier Brunel
On 01/13/15 00:51, Laurent Bercot wrote:
> On 12/01/2015 22:27, Olivier Brunel wrote:
> 
>> I may be wrong, but the way I read the info page about this (4.5.6
>> Directory Search for Link Libraries) I took it that this special
>> handling of processing -lFOO and looking in directories for e.g.
>> libFOO.so then libFOO.a (by default) was only done as such, i.e. to
>> search for the prerequisite (and see if a rebuild is needed if more
>> recent), but when that search fails to find something, then it fails.
>>
>> Because make doesn't know which would be found/used, it has no recipe to
>> build it (e.g. how would it know whether to build libFOO.so or
>> libFOO.a?), as indicated by the error (no rule to make -lexecline)
> 
>  Then how do you explain that it works in the non-parallel case ? The
> sequence of actions you described applies too.

Simply because make process in order, and first there's making the
library file, *then* making the binaries (so linking against the lib),
as per your Makefile. So when it comes to that, the libFOO.a already
exists and there's no issue.

With parallel jobs, the process making the library might still be
running while it'll start the job building the first binary, because it
has no knowledge that it should wait.


>> If you don't have one by the time of next release, may I suggest simply
>> adding a target .NOTPARALLEL so that the -j flag is basically ignored.
>> Doesn't really solve anything, but at least it will always build.
> 
>  Good idea, I'll do that.
> 
> 
>> I see, good to know. So I'll do some reading re: buffered IO, but just
>> to make sure: the buffer_0, buffer_0f1, buffer_1, etc are ways to use
>> stdin, stdout and stderr directly with "pre-allocated" buffers, right?
> 
>  Yes, exactly.
>  You shouldn't use buffer_0f1 unless you're writing a blocking filter.
> buffer_0f1 is just like buffer_0 except that it flushes buffer_1 right
> before attempting to read on stdin - which means that everytime your
> program risks blocking, it makes sure stdout is flushed first. It's
> optimal buffering behaviour for a filter (way better than the standard
> line buffering / full buffering distinction), but it's hackish, so
> make sure you do not use it outside of this very specific case.
> 
> 
>> And, so long as I don't mix e.g. buffer_1 with buffer_1small, I can use
>> them in my code safely?
> 
>  Yes. Just don't mix small with non-small and you'll be fine. :)
> 



Re: building (with) execline

2015-01-12 Thread Olivier Brunel
On 01/12/15 21:27, Laurent Bercot wrote:
> On 12/01/2015 18:30, Olivier Brunel wrote:
> 
>  Hi Olivier,
> 
>> I usually build things with -j4, but with e.g. execline I get an error:
>>
>> make: *** No rule to make target '-lexecline', needed by 'background'.
>> Stop.
>> make: *** Waiting for unfinished jobs
> 
>  Yeah, that's a known problem, and I think the issue is with make.
> 
> 
>> In that case, I think you need to use e.g. libexecline.a instead, so
>> that make can order things properly even with parallel jobs.
>> I tried replacing -lexecline with $(STATIC_LIBS) in deps.mak and I can
>> `make -j4` just fine. Of course this won't work when building with
>> --disable-allstatic, where other changes might be in order.
> 
>  That's the very point: I need a solution that works with both
> libexecline.a and libexecline.so. If it was only me, I'd use
> libexecline.a everywhere, but you know how distributions love
> dynamic linking, so I want to make it easily accessible.
> 
>  GNU make is supposed to replace -lexecline with the static or the
> dynamic version depending on what it finds first, and my Makefile has
> vpath instructions at the beginning, so it's definitely supposed to
> work - but apparently, vpath search doesn't mesh well with make -j,
> and this is not documented in the GNU make manual.

I may be wrong, but the way I read the info page about this (4.5.6
Directory Search for Link Libraries) I took it that this special
handling of processing -lFOO and looking in directories for e.g.
libFOO.so then libFOO.a (by default) was only done as such, i.e. to
search for the prerequisite (and see if a rebuild is needed if more
recent), but when that search fails to find something, then it fails.

Because make doesn't know which would be found/used, it has no recipe to
build it (e.g. how would it know whether to build libFOO.so or
libFOO.a?), as indicated by the error (no rule to make -lexecline)

That's why it works fine for "external" libraries, e.g. -lskarnet, but
not when the library is part of the build process. At least that's how I
read it, hence why I said in this specific case you'd need to use the
actual filename, so there's a rule to make it and make knows to then
wait for it (in case of parallel jobs).
Of course I may be wrong, and this may be an actual bug in make.

>  I will try to come up with a workaround.

If you don't have one by the time of next release, may I suggest simply
adding a target .NOTPARALLEL so that the -j flag is basically ignored.
Doesn't really solve anything, but at least it will always build.


(...)


>> Also, I've made a little thing that works say somewhat like execline's
>> define, and tried to compile it, but it fails. I get a few errors such
>> as:
>> /usr/lib/execline/libexecline.a(el_transform.o): In function
>> `el_transform':
>> (.text+0x112): undefined reference to `satmp'
>> (...)
>> % gcc test.c -static -Wall -lskarnet -lexecline -L/usr/lib/skalibs
>> -L/usr/lib/execline
> 
>  That's a classic linker trap. You need to put the lower-level libraries
> *after* the higher-level libraries on gcc's invocation command line.
> Try "-lexecline -lskarnet" instead of "-lskarnet -lexecline". That
> should solve all your symbol-related problems.

Ah! Thanks a lot, that was my mistake indeed.

> 
> 
>> Yeah, back to stralloc, in your code (e.g. s6) you seem to use it
>> (satmp) at times, but I have no idea how this works. Obviously, it's
>> also something libraries (at least libexecline) might use, so is it
>> meant for app or to be an internal thing that you abuse in your own
>> code? How safe is it to use if library functions can use it?
> 
>  satmp is a global stralloc. The point of satmp is to have a unique
> place for temporary allocations that have to use the heap for some
> reason, instead of making strallocs all over the place and freeing
> them. Reusing the same storage space (and freeing it at the end)
> is cheaper than having series of malloc/free, and it makes it easier
> to keep track of your heap.
> 
>  You can use it safely provided you're using it as a stack:
> * unsigned int oldlen = satmp.len
> * stralloc_catb whatever you want to satmp, whatever is above
> oldlen is free for you to use. What is below oldlen, however,
> is data needed by your caller, and you can't touch it.
> * satmp.len = oldlen
>  Always restore satmp.len when you exit your function, even in
> error conditions.
> 
>  But then again, globals are ugly, so it's probably more trouble
> than it's worth. It's certainly easier and safer for you to make your
> own private stralloc instead of bothering with satmp for some micro
> optimization.
> 

I see, good to know. So I'll do some reading re: buffered IO, but just
to make sure: the buffer_0, buffer_0f1, buffer_1, etc are ways to use
stdin, stdout and stderr directly with "pre-allocated" buffers, right?

And, so long as I don't mix e.g. buffer_1 with buffer_1small, I can use
them in my code safely?



Thanks again,
-j



building (with) execline

2015-01-12 Thread Olivier Brunel
Hi,

I've recently found out about skalibs & related software, and am looking
into it... and found myself with a few questions.

First off, little issue with building things. I've read similar issue on
the archives in this ML (I believe), but I'm not sure whether a solution
was found or not.

I usually build things with -j4, but with e.g. execline I get an error:

make: *** No rule to make target '-lexecline', needed by 'background'.
Stop.
make: *** Waiting for unfinished jobs

Of course with -j1 no such issue. I may be wrong, but I don't think you
can use -lexecline as you do in deps.mak, or maybe at least not when
building said library (unlike -lskarnet).

In that case, I think you need to use e.g. libexecline.a instead, so
that make can order things properly even with parallel jobs.
I tried replacing -lexecline with $(STATIC_LIBS) in deps.mak and I can
`make -j4` just fine. Of course this won't work when building with
--disable-allstatic, where other changes might be in order.


Then, I'm trying things with skalibs & co to see how it works, but there
isn't always (lots of) documentation. For instance, I'm not sure how the
whole buffered IO API work/are meant to be used; Is there any doc about
this somewhere I could read?

Also, I've made a little thing that works say somewhat like execline's
define, and tried to compile it, but it fails. I get a few errors such as:

/usr/lib/execline/libexecline.a(el_transform.o): In function `el_transform':
(.text+0x112): undefined reference to `satmp'


Yeah, back to stralloc, in your code (e.g. s6) you seem to use it
(satmp) at times, but I have no idea how this works. Obviously, it's
also something libraries (at least libexecline) might use, so is it
meant for app or to be an internal thing that you abuse in your own
code? How safe is it to use if library functions can use it?

And back to the original failure, it seems I am forced to use it (and
declare it or include skalibs/skamisc.h) in my own code to make
functions like el_transform() work. Am I missing something/doing
something wrong?

And actually, that's not all:

/usr/lib/execline/libexecline.a(el_transform.o): In function `el_transform':
(.text+0x160): undefined reference to `netstring_decode'


So in order to compile my code, I had to add the following (plus include
skalibs/{skamisc,netstring}.h), only so that functions from libexecline
work:

satmp.len = 0;
void *p = &netstring_decode;
(void) p;

Again, i don't doubt I'm doing something wrong, so I'd appreciate some help.

FYI I'm compiling using:
% gcc test.c -static -Wall -lskarnet -lexecline -L/usr/lib/skalibs
-L/usr/lib/execline

Thanks a lot,
-j