[systemd-devel] [RFC 00/12] Bugfixes for CONFIG_VT=n

[David Herrmann]

Hi

On Wed, Nov 27, 2013 at 11:44 PM, Lennart Poettering
<lennart at poettering.net> wrote:
> On Wed, 27.11.13 19:48, David Herrmann (dh.herrmann at gmail.com) wrote:
>
> Looks pretty good. Commented on the invidiaul patches, but mostly looks
> good.
>
> I am not sure about the whole approach of making this a potentially
> public library though... Especially the monitor stuff sounds so
> specific. Commiting to a stable API for that given that there are very
> few other projects who could ever make use of this sounds dangerous at
> best. Especially since the api exposes bit values for enums and things,
> which makes me especially afraid...

I have no short-term plan to export sd-gfx. It's just like the 3rd
time I happen to write a DRM abstraction and I really don't wanna do
that again. Hence, I thought putting it into sd-gfx would reduce the
burden to export it eventually. Furthermore, as there'll be like 5
daemons using it, it would be nice to have a clean API even if it's
only systemd-internal (it forces one to write better code, imho). Last
but not least, it makes debugging easier if there's a clean cut
between sd-gfx and each application.

But: the current API will never get exported as it is. Obviously, the
monitor (and probably font) is better kept internal. But I see no
reason to add sd-gfx-internal.h / sd-gfx-internal.la now. In case we
ever export it, we would have to decide which parts to split off
(which is a rather mechanical tasks, so no reason to account for it
right now).

So lets just say I hate camel-case object-names so I went for
libsystemd-gfx to have an excuse to use
lower-case-separated-by-underscore names, right? Lets treat sd-gfx as
internal library for now.

A few comments beforehand:
 - regarding logging in libraries: I *want* verbose errors/warnings in
sd-gfx. Especially if modesetting, keymap-compilation or bus-requests
fail, I want some rather verbose messages in the log. Considering that
if a single page-flip fails, I will not abort but continue the modeset
(you don't want your console to abort due to minor timing issues in
the DRM driver, right?), so the application might not even get an
-EXYZ error value. I tried adding a ->log_fn() like libudev, but then
again, sd-gfx is internal so I skipped that and just called log_meta()
directly. Is that fine for now? If not, I can change all these to
log_debug() and make the applications more verbose.
- new0(), zero(), ..., yepp, kay already told me but I forgot.. I will
fix them up.
- moving "out" parameters to the end... will do so.

Thanks for the review. Few more comments on the individual patches.
David