KProcess Line Mode Patch
Ralf Habacker
ralf.habacker at freenet.de
Tue Dec 2 15:23:59 GMT 2008
Andreas Pakulat schrieb:
> On 02.12.08 13:51:57, Ralf Habacker wrote:
>
>> Appended is a proposal for such a class working with the
>> QProcess/KProcess output channel.
>>
>> One question I have is where in the kdelibs such a class could be located ?
>>
> ..
>
>> +#include <QtDebug>
>>
>
> That is probably not wanted in this code.
>
yes, when the code is ready this will be removed
>
>> +qint64 KLineBasedReader::readLine(char * data, qint64 maxSize)
>> +{
>> + qint64 ret = m_p->readLine(data, maxSize);
>> + if (ret <= 0)
>> + return ret;
>> + if (data[strlen(data)-2] == '\x0d' && data[strlen(data)-1] == '\x0a')
>>
>
> That won't happen, QIODevice will already replace windows line-endings with
> unix-ones (unless you don't use QIODevice::Text, in which case you
> shouldn't be reading lines).
>
That would mean to override the related QIODevice Setting in
KProcessPrivate
openMode(QIODevice::ReadWrite)
>
>> + data[strlen(data)-2] = '\0';
>> + else if (data[strlen(data)-1] == '\x0a')
>> + data[strlen(data)-1] = '\0';
>> + return ret;
>> +}
>>
>
> With the above changes, what exactly is the benefit of this class?
I quote from Thiago's answer
"What I'd recommend instead is a line-based reader that works on top of a plain
QIODevice. That way, you can plug it on a QTcpSocket, QLocalSocket,
QNetworkReply or KTcpSocket objects as well. "
> IMHO it doesn't provide any real value compared to listening for the readyRead()
> signal from QIODevice+checking for canReadLine and then reading the line
> into a char* or even a QByteArray.
>
if it so easy then of source there is no need for such class.
> So where are the two apps that use this class? And why do they use it?
>
When searching for readyReadStandardOutput on
http://lxr.kde.org/ident?i=readyReadStandardOutput shows I got 130
references in 81 files.
Inspecting them shows several problematic implementations of our related
topic
http://lxr.kde.org/source/KDE/kdeadmin/ksystemlog/src/lib/processOutputLogFileReader.cpp#117
196 <http://lxr.kde.org/source/KDE/kdeadmin/ksystemlog/src/lib/processOutputLogFileReader.cpp#196> void ProcessOutputLogFileReader <http://lxr.kde.org/ident?i=ProcessOutputLogFileReader>::logFileModified <http://lxr.kde.org/ident?i=logFileModified>() {
197 <http://lxr.kde.org/source/KDE/kdeadmin/ksystemlog/src/lib/processOutputLogFileReader.cpp#197> Q_D(ProcessOutputLogFileReader <http://lxr.kde.org/ident?i=ProcessOutputLogFileReader>);
198 <http://lxr.kde.org/source/KDE/kdeadmin/ksystemlog/src/lib/processOutputLogFileReader.cpp#198>
199 <http://lxr.kde.org/source/KDE/kdeadmin/ksystemlog/src/lib/processOutputLogFileReader.cpp#199> logDebug <http://lxr.kde.org/ident?i=logDebug>() << "Content available on process output..." << endl <http://lxr.kde.org/ident?i=endl>;
200 <http://lxr.kde.org/source/KDE/kdeadmin/ksystemlog/src/lib/processOutputLogFileReader.cpp#200>
201 <http://lxr.kde.org/source/KDE/kdeadmin/ksystemlog/src/lib/processOutputLogFileReader.cpp#201> //New added lines
202 <http://lxr.kde.org/source/KDE/kdeadmin/ksystemlog/src/lib/processOutputLogFileReader.cpp#202> QByteArray <http://lxr.kde.org/ident?i=QByteArray> bytesOutput = d <http://lxr.kde.org/ident?i=d>->process <http://lxr.kde.org/ident?i=process>->readAllStandardOutput();
203 <http://lxr.kde.org/source/KDE/kdeadmin/ksystemlog/src/lib/processOutputLogFileReader.cpp#203> d <http://lxr.kde.org/ident?i=d>->buffer <http://lxr.kde.org/ident?i=buffer>.append <http://lxr.kde.org/ident?i=append>(QString <http://lxr.kde.org/ident?i=QString>(bytesOutput));
204 <http://lxr.kde.org/source/KDE/kdeadmin/ksystemlog/src/lib/processOutputLogFileReader.cpp#204>
Ouch !!!
358 <http://lxr.kde.org/source/KDE/kdebase/apps/konqueror/settings/konqhtml/pluginopts.cpp#358> void KPluginOptions <http://lxr.kde.org/ident?i=KPluginOptions>::progress <http://lxr.kde.org/ident?i=progress>()
359 <http://lxr.kde.org/source/KDE/kdebase/apps/konqueror/settings/konqhtml/pluginopts.cpp#359> {
360 <http://lxr.kde.org/source/KDE/kdebase/apps/konqueror/settings/konqhtml/pluginopts.cpp#360> // we do not know if the output array ends in the middle of an utf-8 sequence
361 <http://lxr.kde.org/source/KDE/kdebase/apps/konqueror/settings/konqhtml/pluginopts.cpp#361> m_output <http://lxr.kde.org/ident?i=m_output> += nspluginscan <http://lxr.kde.org/ident?i=nspluginscan>->readAllStandardOutput();
362 <http://lxr.kde.org/source/KDE/kdebase/apps/konqueror/settings/konqhtml/pluginopts.cpp#362> QString <http://lxr.kde.org/ident?i=QString> line <http://lxr.kde.org/ident?i=line>;
363 <http://lxr.kde.org/source/KDE/kdebase/apps/konqueror/settings/konqhtml/pluginopts.cpp#363> int pos <http://lxr.kde.org/ident?i=pos>;
364 <http://lxr.kde.org/source/KDE/kdebase/apps/konqueror/settings/konqhtml/pluginopts.cpp#364> while ((pos <http://lxr.kde.org/ident?i=pos> = m_output <http://lxr.kde.org/ident?i=m_output>.indexOf <http://lxr.kde.org/ident?i=indexOf>('\n')) != -1) {
365 <http://lxr.kde.org/source/KDE/kdebase/apps/konqueror/settings/konqhtml/pluginopts.cpp#365> line <http://lxr.kde.org/ident?i=line> = QString <http://lxr.kde.org/ident?i=QString>::fromLocal8Bit(m_output <http://lxr.kde.org/ident?i=m_output>, pos <http://lxr.kde.org/ident?i=pos> + 1);
366 <http://lxr.kde.org/source/KDE/kdebase/apps/konqueror/settings/konqhtml/pluginopts.cpp#366> m_output <http://lxr.kde.org/ident?i=m_output>.remove <http://lxr.kde.org/ident?i=remove>(0, pos <http://lxr.kde.org/ident?i=pos> + 1);
367 <http://lxr.kde.org/source/KDE/kdebase/apps/konqueror/settings/konqhtml/pluginopts.cpp#367> }
368 <http://lxr.kde.org/source/KDE/kdebase/apps/konqueror/settings/konqhtml/pluginopts.cpp#368> m_progress <http://lxr.kde.org/ident?i=m_progress>->progressBar <http://lxr.kde.org/ident?i=progressBar>()->setValue <http://lxr.kde.org/ident?i=setValue>(line <http://lxr.kde.org/ident?i=line>.trimmed <http://lxr.kde.org/ident?i=trimmed>().toInt <http://lxr.kde.org/ident?i=toInt>());
369 <http://lxr.kde.org/source/KDE/kdebase/apps/konqueror/settings/konqhtml/pluginopts.cpp#369> }
370 <http://lxr.kde.org/source/KDE/kdebase/apps/konqueror/settings/konqhtml/pluginopts.cpp#370>
http://lxr.kde.org/source/extragear/office/kile/src/dialogs/postscriptdialog.h#209
064 <http://lxr.kde.org/source/extragear/office/kile/src/dialogs/postscriptdialog.h#064> Q_SIGNALS:
065 <http://lxr.kde.org/source/extragear/office/kile/src/dialogs/postscriptdialog.h#065> void output <http://lxr.kde.org/ident?i=output>(const QString <http://lxr.kde.org/ident?i=QString> &);
http://lxr.kde.org/source/extragear/office/kile/src/dialogs/postscriptdialog.cpp#209
222 <http://lxr.kde.org/source/extragear/office/kile/src/dialogs/postscriptdialog.cpp#222> void PostscriptDialog <http://lxr.kde.org/ident?i=PostscriptDialog>::slotProcessOutput <http://lxr.kde.org/ident?i=slotProcessOutput>()
223 <http://lxr.kde.org/source/extragear/office/kile/src/dialogs/postscriptdialog.cpp#223> {
224 <http://lxr.kde.org/source/extragear/office/kile/src/dialogs/postscriptdialog.cpp#224> emit <http://lxr.kde.org/ident?i=emit>(output <http://lxr.kde.org/ident?i=output>(m_proc <http://lxr.kde.org/ident?i=m_proc>->readAllStandardOutput()));
225 <http://lxr.kde.org/source/extragear/office/kile/src/dialogs/postscriptdialog.cpp#225> emit <http://lxr.kde.org/ident?i=emit>(output <http://lxr.kde.org/ident?i=output>(m_proc <http://lxr.kde.org/ident?i=m_proc>->readAllStandardError()));
226 <http://lxr.kde.org/source/extragear/office/kile/src/dialogs/postscriptdialog.cpp#226> }
227 <http://lxr.kde.org/source/extragear/office/kile/src/dialogs/postscriptdialog.cpp#227>
Ouch !!
KHelpcenter index generating using khc_indexbuilder -> see void
KCMHelpCenter::slotReceivedStdout()
kdebase/apps/konqueror/settings/konqhtml/pluginopts.cpp
void KPluginOptions::progress()
359 {
360 // we do not know if the output array ends in the middle of an
utf-8 sequence
361 m_output += nspluginscan->readAllStandardOutput();
362 QString line;
363 int pos;
364 while ((pos = m_output.indexOf('\n')) != -1) {
365 line = QString::fromLocal8Bit(m_output, pos + 1);
366 m_output.remove(0, pos + 1);
367 }
368 m_progress->progressBar()->setValue(line.trimmed().toInt());
369 }
kdebase/runtime/drkonqi/backtrace.cpp
113 void BackTrace::slotReadInput()
114 {
115 // we do not know if the output array ends in the middle of an
utf-8 sequence
116 m_output += m_proc->readAllStandardOutput();
117
118 int pos;
119 while ((pos = m_output.indexOf('\n')) != -1)
120 {
121 QString line = QString::fromLocal8Bit(m_output, pos + 1);
122 m_output.remove(0, pos + 1);
123
124 m_strBt.append(line);
125 emit append(line);
126 }
127 }
128
087 void SensorShellAgent::msgRcvd( )
088 {
089 QByteArray buffer = mDaemon->readAllStandardOutput();
090 mRetryCount = 3; //we received an answer, so reset our retry count
back to 3
091 processAnswer( buffer.constData(), buffer.size());
092 }
093
094 void SensorShellAgent::errMsgRcvd( )
095 {
096 QByteArray buffer = mDaemon->readAllStandardOutput();
097
098 // Because we read the error buffer in chunks, we may not have a
proper utf8 string.
099 // We should never get input over stderr anyway, so no need to
worry too much about it.
100 // But if this is extended, we will need to handle this better
101 QString buf = QString::fromUtf8( buffer );
102
103 kDebug(1215) << "SensorShellAgent: Warning, received text over
stderr!"
104 << endl << buf << endl;
105 }
106
Ouch !!!
http://lxr.kde.org/source/playground/graphics/klotz/src/app/globals.cpp#103
151 <http://lxr.kde.org/source/playground/graphics/klotz/src/app/globals.cpp#151> void KlotzGlobals <http://lxr.kde.org/ident?i=KlotzGlobals>::dbUpdateStatus <http://lxr.kde.org/ident?i=dbUpdateStatus>()
152 <http://lxr.kde.org/source/playground/graphics/klotz/src/app/globals.cpp#152> {
153 <http://lxr.kde.org/source/playground/graphics/klotz/src/app/globals.cpp#153> if (!dbDialog_ <http://lxr.kde.org/ident?i=dbDialog_>) {
154 <http://lxr.kde.org/source/playground/graphics/klotz/src/app/globals.cpp#154> dbDialog_ <http://lxr.kde.org/ident?i=dbDialog_> = new KProgressDialog <http://lxr.kde.org/ident?i=KProgressDialog>(0L, i18n <http://lxr.kde.org/ident?i=i18n>("Scanning"), i18n <http://lxr.kde.org/ident?i=i18n>("<qt><p align=center>Klotz is now creating database from LDraw part library in your system. Please wait...<br/>%1</p></qt>", QString <http://lxr.kde.org/ident?i=QString>()));
155 <http://lxr.kde.org/source/playground/graphics/klotz/src/app/globals.cpp#155> dbDialog_ <http://lxr.kde.org/ident?i=dbDialog_>->setAutoClose <http://lxr.kde.org/ident?i=setAutoClose>(true);
156 <http://lxr.kde.org/source/playground/graphics/klotz/src/app/globals.cpp#156> dbDialog_ <http://lxr.kde.org/ident?i=dbDialog_>->showCancelButton <http://lxr.kde.org/ident?i=showCancelButton>(false);
157 <http://lxr.kde.org/source/playground/graphics/klotz/src/app/globals.cpp#157> dbDialog_ <http://lxr.kde.org/ident?i=dbDialog_>->show <http://lxr.kde.org/ident?i=show>();
158 <http://lxr.kde.org/source/playground/graphics/klotz/src/app/globals.cpp#158> }
159 <http://lxr.kde.org/source/playground/graphics/klotz/src/app/globals.cpp#159>
160 <http://lxr.kde.org/source/playground/graphics/klotz/src/app/globals.cpp#160> dbUpdater_ <http://lxr.kde.org/ident?i=dbUpdater_>->setReadChannel(KProcess <http://lxr.kde.org/ident?i=KProcess>::StandardOutput <http://lxr.kde.org/ident?i=StandardOutput>);
161 <http://lxr.kde.org/source/playground/graphics/klotz/src/app/globals.cpp#161>
162 <http://lxr.kde.org/source/playground/graphics/klotz/src/app/globals.cpp#162> QStringList <http://lxr.kde.org/ident?i=QStringList> message <http://lxr.kde.org/ident?i=message> = QString <http://lxr.kde.org/ident?i=QString>(dbUpdater_ <http://lxr.kde.org/ident?i=dbUpdater_>->readAll <http://lxr.kde.org/ident?i=readAll>()).trimmed <http://lxr.kde.org/ident?i=trimmed>().split <http://lxr.kde.org/ident?i=split>('\n');
One interesting thing is that there are already floating around line
orientated KProcess implementations like in
http://lxr.kde.org/source/playground/sysadmin/keep/common/rdblistener.cpp#044
044 <http://lxr.kde.org/source/playground/sysadmin/keep/common/rdblistener.cpp#044> void RDBListener <http://lxr.kde.org/ident?i=RDBListener>::readyReadStandardOutput <http://lxr.kde.org/ident?i=readyReadStandardOutput>()
045 <http://lxr.kde.org/source/playground/sysadmin/keep/common/rdblistener.cpp#045> {
046 <http://lxr.kde.org/source/playground/sysadmin/keep/common/rdblistener.cpp#046> proc <http://lxr.kde.org/ident?i=proc>->setReadChannel(QProcess <http://lxr.kde.org/ident?i=QProcess>::StandardOutput <http://lxr.kde.org/ident?i=StandardOutput>);
047 <http://lxr.kde.org/source/playground/sysadmin/keep/common/rdblistener.cpp#047> QByteArray <http://lxr.kde.org/ident?i=QByteArray> output <http://lxr.kde.org/ident?i=output>;
048 <http://lxr.kde.org/source/playground/sysadmin/keep/common/rdblistener.cpp#048> while(!(output <http://lxr.kde.org/ident?i=output> = proc <http://lxr.kde.org/ident?i=proc>->readLine <http://lxr.kde.org/ident?i=readLine>()).isEmpty <http://lxr.kde.org/ident?i=isEmpty>()) {
049 <http://lxr.kde.org/source/playground/sysadmin/keep/common/rdblistener.cpp#049> kDebug <http://lxr.kde.org/ident?i=kDebug>() << "Message: " << output <http://lxr.kde.org/ident?i=output>;
050 <http://lxr.kde.org/source/playground/sysadmin/keep/common/rdblistener.cpp#050> m_stdOut <http://lxr.kde.org/ident?i=m_stdOut>.append <http://lxr.kde.org/ident?i=append>(output <http://lxr.kde.org/ident?i=output>);
051 <http://lxr.kde.org/source/playground/sysadmin/keep/common/rdblistener.cpp#051> }
052 <http://lxr.kde.org/source/playground/sysadmin/keep/common/rdblistener.cpp#052> }
053 <http://lxr.kde.org/source/playground/sysadmin/keep/common/rdblistener.cpp#053>
here is stopped the research ... there are probably more cases like the
above mentioned examples
>> +QString KLineBasedReader::readLine (qint64 maxSize)
>> +{
>> + QString data = m_p->readLine(maxSize);
>>
>
> Again the encoding problem, readLine returns a QByteArray, you can't just
> convert that to a QString without knowing the right encoding.
>
>
This shows once more that there is a need for a such helper class to
avoid broken code
Ralf
More information about the kde-core-devel
mailing list