[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-devel
Subject:    Re: resurrecting kwrited
From:       "George Kiagiadakis" <kiagiadakis.george () gmail ! com>
Date:       2008-10-13 13:40:57
Message-ID: ba4a6e220810130640s47fb5e59uef48171ae79a414e () mail ! gmail ! com
[Download RAW message or body]

Hi ossi, thanks for the reply.

2008/10/13 Oswald Buddenhagen <ossi@kde.org>:
> 054d8a404da937f9e0e5fdf0ef189114ed10b812
> - no point in checking for <= 0 in block_in(), you'd get different
> signals

You mean no point for checking buf.isEmpty()... That's right, but I
wasn't very sure how QIODevice behaves and I wanted to stick to the
old code as close as possible so that I don't get any regression.

> - readAll() instead of the arbitrary size read
> why do you bother with the much bigger higher-level class if you don't
> use any of its features anyway? ;)

True, I did that for the same reason as above.

> - see http://techbase.kde.org/Development/Tutorials/Common_Programming_Mistakes#Reading_QString_from_a_KProcess
>  see kdm/kfrontend/kconsole.cpp:slotData()

I fail to understand this. I am aware of that "common programming
mistake", but I am not reading from a process here, am I? Also, this
slotData() doesn't do anything different, it just uses read(2) to read
data into a buffer. Maybe you think I should revert that commit? No
problem, but well, I prefer the Qt way from system calls :)

> 18c79315a8bfa2566a6744cb7b8e521183d9d1c0
> - this is actually a bit silly. the tty output should be probably a
> shorter, not internationalized qWarning()

Yeah, I was already thinking to change that into a kDebug(). It's a
debugging message after all.

> 7f8825dff376b5df7f5fbd8afa298a29f7084d04
> - hmpf. this is bad bad bad. oh, well.

Why? kwrited is completely useless without libutempter and it justs
wastes memory. Actually I did that because there was a TODO item in
kwrited.cpp saying that kwrited should not build if libutempter is not
available. I just implemented that.

> 9f9be009ba0c74ab667531a6cbfc3bbd2454ab11
> - two remove()s with single chars would be *way* faster than the regexp

OK.

> d8e38b9b05b274b36032ca7acdc96cfad99f3b27
> - misfeature, imho

Why? If the number of broadcast messages reveived is > 2, any new
messages do not appear inside the current viewport (because they are
normally appended to the bottom), so the user would just get a popup
showing some old message and would have to scroll to the bottom
manually.

Btw, I noticed that the messages are actually inserted in the place
where the text cursor is (reasonable), but the position of the text
cursor can change if you click somewhere, although the textedit is
readonly. QTextEdit bug?

> > The knotify branch introduces the feature to use KNotify instead of
> > popping up the kwrited window on every new message. [...]
> > I thought it would be better to drop completely the kwrited window and
> > use ONLY knotify. What do you think of that?
> > 
> reasonable.

Ok, so far I have 2 people that agree with me, so I will implement
that today. (so, forget all the KTextEdit issues above)

> > The no-utempter branch introduces a workaround to my problem. [...]
> > 
> i think we already talked about that on irc?

Yep, we did, and you actually helped me to understand this problem
better. Thanks for that :)

> hmm, that's interesting - KApplication lost its "i refuse to run with
> setuid/setgid" feature on the way to kde 4. so the rest of my thoughts
> on that matter becomes a bit irrelevant. anyway:

Really? konsole does not start in setgid mode, however.

> - it sucks to repeat the utempter check for every use case. we need some
> global flag KPtyNeedsSetGid which can be used in cmakefiles. or simply
> a cmake postinstall macro apply_kpty_setgid_utmp() which is possibly a
> noop

I agree with that, I have already repeated this test in two
CMakeLists.txt files and it doesn't seem so nice. Also I did not write
any cmake code to setgid kwrited as I couldn't find how to do chown
from cmake.

> - KPty should handle restoring privs for login() and dropping them again
> automatically. libkpty would have a global constructor to drop the
> privs on startup in the first place.

I agree, please implement that :P
 
> > Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<


[prev in list] [next in list] [prev in thread] [next in thread]