[systemd-devel] eudev fork and patches there

Zbigniew Jędrzejewski-Szmek zbyszek at in.waw.pl
Wed Dec 19 10:56:28 PST 2012


On Wed, Dec 19, 2012 at 02:45:59PM +0100, Lennart Poettering wrote:
> On Mon, 17.12.12 12:17, Zbigniew Jędrzejewski-Szmek (zbyszek at in.waw.pl) wrote:
> 
> > Hi,
> > 
> > eudev has made a project annoucement [1], and I thought it would be
> > worthwhile to go through their patches and cherry-pick. I've now done
> > that (until cc5c144a70fc37e 'Merge pull request #32') and the results
> > regarding patches which can be used directly are not very impressive:
> > one patch to fix clang warnings. There are quite a few
> > build/compiler-warning in their tree, but they just seem to be fixes
> > for problems introduced with their changes.
> > 
> > Potentially usefull would also be the patch c189ab 'Fix unused result
> > warnings', but it would be good if somebody familar with the code took
> > a look if it is enough to print warnings (or even if they should be
> > printed) or some more definite action should be taken.
> 
> Looking over this I am actually quite sure that these invocations do not
> need to be checked specifically. i.e. the errors can be safely ignored,
> or are detected later on the control flow anyway.
Great. I was worried about the lockf's, they are a bit tricky.

> Given that this is udev code, Kay probably should have a closer look
> before we decide to ignore this entirely.
> 
> (In contrast to checking frwite() and friends on every single invocation
> it is usually nicer to just check ferror() after one is done with
> everything one had to do...)
> 
> > Also potentially usefull are the changes to allow 'make distcheck'
> > without gtk-doc, but they are spread out over multiple commits in a
> > messy way, so could only be used as inspiration.
> 
> Well, "make distcheck" is supposed to be invoked right before doing a
> release. As such it damn well should check if the gtk-doc stuff builds
> correctly, as that's part of the release. Hence I think this change is
> counterproductive...
I like to run 'make distcheck' after any changes which could impact the
build, just to be safe. And sometime this happens in a build configured
without --with-gtk-doc (or whatever this option is called), and I have
to first look up the option name, then reconfigure and build, and then
run distcheck. Other configure options do not behave like that and 'make
distcheck' works when they are set or not... So actually, for me, a
change in this direction would be productive.

> > There's also nice patch bfc850 'Add fallback path when accept4() is
> > not available.' Do we care about kernels < 2.6.28? (This version is
> > according to man accept4(2), patch text mentions different versions.)
> 
> I am pretty sure that if people care about this, they should patch
> proper accept4 support into their libc rather than patching all
> consumers of libc to work without it. Fix the issues where they are,
> it's less work and more correct anyway.
Of course, technically there is no discussion. Nevertheless,
accepting some patches like that would make like easier for some
poeple, and also help avoid pointless forks. Maybe the energy going
into eudev would be better spent on patches for systemd/udev.

Zbyszek


More information about the systemd-devel mailing list