Hi On Wed, Nov 27, 2013 at 11:44 PM, Lennart Poettering <lenn...@poettering.net> wrote: > On Wed, 27.11.13 19:48, David Herrmann (dh.herrm...@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 _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel