[systemd-devel] [systemd-commits] 9 commits - configure.ac .gitignore Makefile.am Makefile-man.am man/systemd-fsckd.service.xml man/systemd-fsck at .service.xml po/de.po po/el.po po/fr.po po/hu.po po/it.po po/pl.po po/POTFILES.in po/pt_BR.po po/ru.po po/sv.po po/uk.po src/fsck src/fsckd src/shared test/mocks units/.gitignore units/systemd-fsckd.service.in units/systemd-fsckd.socket units/systemd-fsck-root.service.in units/systemd-fsck at .service.in

[Martin Pitt]

Hey Lennart,

Lennart Poettering [2015-03-09 18:39 +0100]:
> I looked in more detail at the fsckd code that was commited a few
> weeks ago, and I cannot say I liked what I saw. The code still has all
> kinds of issues, including memory corruptions, fd handling errors, and
> tons and tons of incorrect error handling (I think more times the code
> passes an incorrect error to log_xyz_errno() than a correct one!).

Argh, thanks for spotting them!

> And there are conceptional issues with the code too (for example,
> it's not OK to keep /dev/console open, this breaks the SAK key
> logic...)

Ah, good point. This isn't really new (previously, s-fsck kept it open
all the time), so we weren't aware of that SAK issue, but of course
that should be fixed.

> Please, be more careful with complex code like this, this needs more
> rounds of review before something like this can be merged...

Okay. It went through 4 rounds of review on the ML and several folks
commented, there were no more outstanding/unaddressed review comments,
and it seemed to me that we had consensus on IRC to land this now.
Procedurally, what should I/we do next time before landing such large
patches? Wait for an explicit ack on the ML from you?

Thanks and sorry for the hassle,

Martin
-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)