D17737: [CopyJob] Create clones in btrf/xfs mount

ossi added a comment. In D17737#517736 , @thiago wrote: > QFile::copy already implements FICLONE, sendfile() and Darwin's clonefile(). yes, and an api that is utterly unsuitable for interactive applications. ;) if you just meant it as

D11236: [KCrash] Establish socket to allow change of ptracer

ossi accepted this revision. ossi added a comment. This revision is now accepted and ready to land. here too, it should be "ancestor" not "parent" for extra correctness. amending the code comment is optional, too. INLINE COMMENTS > kcrash.cpp:662 > +// For now that will be DrKonqi,

D11235: [DrKonqi] Request change of ptrace scope from KCrash

ossi accepted this revision. ossi added a comment. This revision is now accepted and ready to land. note that for extra correctness it should be "ancestor" not "parent". REPOSITORY R871 DrKonqi BRANCH ptracer REVISION DETAIL https://phabricator.kde.org/D11235 To: croick, #plasma_works

D11236: [KCrash] Establish socket to allow change of ptracer

ossi added a comment. i said "3rd line", not "3rd case". it's about the ancestry; see the other change. REPOSITORY R285 KCrash REVISION DETAIL https://phabricator.kde.org/D11236 To: croick, #frameworks, ossi Cc: dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh, ngra

D11236: [KCrash] Establish socket to allow change of ptracer

ossi added a comment. the 3rd line is still wrong ... REPOSITORY R285 KCrash REVISION DETAIL https://phabricator.kde.org/D11236 To: croick, #frameworks, ossi Cc: dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh, ngraham, bruns

D11236: [KCrash] Establish socket to allow change of ptracer

ossi added a comment. right, threads, i didn't think of that. i suppose it might make sense to pthread_kill() from within kcrash to freeze the other threads, but that will be non-atomic, and i wonder whether it wouldn't have undesirable interactions with the attached debugger and/or exiting

D11236: [KCrash] Establish socket to allow change of ptracer

ossi added inline comments. INLINE COMMENTS > kcrash.cpp:658 > +// when launching drkonqi. Note that DrKonqi will SIGSTOP this > process in the meantime > +// and only send SIGCONT when it is about to attach a debugger. > #ifdef Q_OS_LINUX ok. but come to think of it, maybe tha

D11235: [DrKonqi] Request change of ptrace scope from KCrash

ossi added a comment. In D11235#395021 , @croick wrote: > What do you mean? The (internal) debugger is a child of the debuggee. yes, that's why you need setPtracer() - if crashtest was *under* gdb, that would be unnecessary. ;) (i'm sti

D11236: [KCrash] Establish socket to allow change of ptracer

ossi added a comment. commit message inverted here as well. INLINE COMMENTS > kcrash.cpp:658 > +// when launching drkonqi. Note that DrKonqi will SIGSTOP this > process in the meantime > +// and only send SIGCONT when it is to be attached by a debugger. > #ifdef Q_OS_LINUX

D11235: [DrKonqi] Request change of ptrace scope from KCrash

ossi added a comment. you got the parent-child ordering wrong in the commit message. ^^ INLINE COMMENTS > ptracer.cpp:61 > +char msg[msize], rmsg[msize]; > +int bytes = 0, r; > +sprintf(msg, "%lld", debuggerpid); this looks stupid, i'd swap the order. REPOSITORY R8

D11235: [DrKonqi] Request change of ptrace scope from KCrash

ossi added a comment. will also need to wait for commit message update, like the other change. INLINE COMMENTS > ptracer.cpp:92 > + > +qCWarning(DRKONQI_LOG) << "debugged process did not acknowledge setting > ptracer to" << debuggerpid; > +close(sfd); not wrong per se, but mildly mi

D11235: [DrKonqi] Request change of ptrace scope from KCrash

ossi requested changes to this revision. ossi added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > ptracer.cpp:65 > +bytes += r; > +else if (r == -1 && !(errno == EINTR)) > +break; you really could just use != here ..

D11236: [KCrash] Establish socket to allow change of ptracer

ossi added a comment. In D11236#394363 , @croick wrote: > Actually the point I removed does not seem to be true any longer. I'm almost certain, that the `prctrl(PR_SET_PTRACER, ...)` was not required until some time ago when starting DrKonqi as

D11235: [DrKonqi] Request change of ptrace scope from KCrash

ossi requested changes to this revision. ossi added a comment. This revision now requires changes to proceed. mark handled issues as done here as well. INLINE COMMENTS > ptracer.cpp:47 > + > QStringLiteral("%1/kcrash_%2").arg(QStandardPaths::writableLocation(QStandardPaths::RuntimeLoc

D11236: [KCrash] Establish socket to allow change of ptracer

ossi requested changes to this revision. ossi added a comment. This revision now requires changes to proceed. mostly minor issues left. please explicitly mark all handled issues as done - you'll notice on the way that you didn't address some of them. you adjusted the summary kinda th

D11236: [KCrash] Establish socket to allow change of ptrace scope

ossi added a comment. In D11236#393789 , @ossi wrote: > detaching via double fork() "protects" the child process, > but does *not* break the permission inheritance. ok, i take that back and claim the opposite - i misinterpreted

D11236: [KCrash] Establish socket to allow change of ptrace scope

ossi added a comment. In D11236#393720 , @croick wrote: > Actually it's only the internal process that creates the backtraces, that is a descendant of DrKonqi. All debuggers that are launched via the "Debug" button are processes on their own (to

D11235: [DrKonqi] Request change of ptrace scope from KCrash

ossi requested changes to this revision. ossi added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > debuggerlaunchers.cpp:57 > if ( pid > 0 ) { > +queryPtrace(pid); > m_monitor->startMonitoring(pid); according to my reading of the documenta

D11236: [KCrash] Establish socket to allow change of ptrace scope

ossi requested changes to this revision. ossi added a comment. This revision now requires changes to proceed. yay, i finally have "some" time for this. ^^ it took me a while to get a coherent picture of the problem and solution, because your description unnecessarily dwells on the irrelev

D12745: Unify API for file descriptor sharing

ossi added a comment. \o/ ;) REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12745 To: chinmoyr, dfaure, ossi Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D12291: Accept file descriptor only from root owned process

ossi added inline comments. INLINE COMMENTS > chinmoyr wrote in fdreceiver.cpp:89 > > i don't see why that would be horrible > > I meant adding "acceptConnection = true;" after #warning would look weird. > Obviously that's not even an issue and I shouldn't have mentioned it. > > There is a dis

D12291: Accept file descriptor only from root owned process

ossi added inline comments. INLINE COMMENTS > chinmoyr wrote in fdreceiver.cpp:89 > I don't think making acceptConnection unconditionally true is a good idea. At > least it will make this patch look horrible. > Since getsocketopt conforms to posix, how about we check for __ unix __ , and > posi

D12291: Accept file descriptor only from root owned process

ossi added inline comments. INLINE COMMENTS > fdreceiver.cpp:89 > +#else > +#warning Cannot get socket credentials! > +#endif i wonder whether that shouldn't come with an unconditional acceptConnection = true; then - now the compilation succeeds, but it will fail to operate. given that the who

D12291: Accept file descriptor only from root owned process

ossi accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D12291 To: chinmoyr, #frameworks, dfaure, ossi Cc: kde-frameworks-devel, ossi, michaelh, ngraham, bruns

D12745: Unify API for file descriptor sharing

ossi accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D12745 To: chinmoyr, dfaure, ossi Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D9966: [KIO] Fix issues with sharing of file descriptor

ossi added a comment. Restricted Application added a subscriber: kde-frameworks-devel. this dependency tree is a mess. please remove deps to abandoned changes or unabandon what you actually still need, and linearize the history. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde

D12744: Add null pointer check when creating SocketAddress

ossi accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D12744 To: chinmoyr, dfaure, ossi Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D10410: Move the task of cleaning up of socket file to file ioslave and FdReceiver

ossi accepted this revision. ossi added a comment. This revision is now accepted and ready to land. note that the commit message needs a minor adjustment now. INLINE COMMENTS > chinmoyr wrote in fdreceiver.cpp:57 > So shall I commit this change separately right after pushing this patch? Or >

D12745: Unify API for file descriptor sharing

ossi added a comment. but so does using raw pointers. as the stl is available here anyway, it seems like the preferable abstraction layer. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12745 To: chinmoyr, dfaure, ossi Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D12745: Unify API for file descriptor sharing

ossi added a comment. why aren't you standardizing on std::string? that's cleaner than raw char pointers. i know we discussed this before to some degree, but i don't remember the particulars. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12745 To: chinmoyr, dfaure,

D12744: Add null pointer check when creating SocketAddress

ossi requested changes to this revision. ossi added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > fdreceiver.cpp:34 > { > +const SocketAddress addr(m_path.toLocal8Bit().constData()); > +if (!addr.address()) { it would be more elegant to use m_path.toL

D10411: Create socket file in user's runtime directory

ossi accepted this revision. Restricted Application added a subscriber: kde-frameworks-devel. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D10411 To: chinmoyr, #frameworks, ossi Cc: kde-frameworks-devel, dfaure, michaelh, ngraham, bruns

D10410: Move the task of cleaning up of socket file to file ioslave and FdReceiver

ossi requested changes to this revision. ossi added a comment. This revision now requires changes to proceed. did you make sure that this is the only place where SocketAddress is used? INLINE COMMENTS > fdreceiver.cpp:41 > const SocketAddress addr(path.toStdString()); > if (bind(m_s

D12291: Accept file descriptor only from root owned process

ossi added a comment. as i certainly mentioned somewhere else already, this is redundant with putting the socket in a safe place. but fair enough ... INLINE COMMENTS > fdreceiver.cpp:67 > if (client > 0) { > -FDMessageHeader msg; > -if (::recvmsg(client, msg.message(), 0

D10273: Create proper SocketAddress

ossi requested changes to this revision. ossi added a comment. This revision now requires changes to proceed. note that there are also unaddressed comments from previous rounds. INLINE COMMENTS > sharefd_p.h:50 > { > -return reinterpret_cast(&addr); > +return (addr.sun_pa

D10409: In linux don't use abstract socket to share file descriptor

ossi accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D10409 To: chinmoyr, #frameworks, ossi Cc: dfaure, michaelh, ngraham, bruns

D10410: Move the task of cleaning up of socket file to file ioslave and FdReceiver

ossi requested changes to this revision. ossi added a comment. This revision now requires changes to proceed. still needs update REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10410 To: chinmoyr, #frameworks, ossi, dfaure Cc: michaelh, ngraham, bruns

D10411: Create socket file in user's runtime directory

ossi accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D10411 To: chinmoyr, #frameworks, ossi Cc: dfaure, michaelh, ngraham, bruns

D10409: In linux don't use abstract socket to share file descriptor

ossi added a comment. not sure why; the changes are semantically separate. my suggestion was to put this before D10273 , thus reducing the latter's size. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10409 To: chinmoyr, #framew

D10409: In linux don't use abstract socket to share file descriptor

ossi added a comment. that contradicts the comments i added to both reviews. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10409 To: chinmoyr, #frameworks, ossi Cc: dfaure, michaelh

D10409: In linux don't use abstract socket to share file descriptor

ossi added a comment. the idea is that you can do directory-based access controls on file-based sockets, while the abstract namespace has no controls. otoh, only linux has the abstract namespace, and it supports peer credential verification as well, so this doesn't actually add any security

D10273: Create proper SocketAddress

ossi added inline comments. INLINE COMMENTS > dfaure wrote in fdsender.cpp:24 > In that case I don't understand why SocketAddress takes a QByteArray and not > a std::string Because ossi suggested it to unify the API and avoid one > conversion to std::string in fdereceiver? But then we have

D9966: [KIO] Fix issues with sharing of file descriptor

ossi added a comment. looks good, though technically speaking this still fixes two separate issues. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D9966 To: chinmoyr, #frameworks, thiago, dfaure, ossi Cc: ngraham, fvogt, lbeltrame, dfaure, michaelh

D10410: Move the task of cleaning up of socket file to file ioslave and FdReceiver

ossi added inline comments. INLINE COMMENTS > chinmoyr wrote in fdreceiver.cpp:62 > I didn't want to include QFile. That's the only reason. uhm, you're still using it in this very line, just differently ;) REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D10

D10410: Move the task of cleaning up of socket file to file ioslave and FdReceiver

ossi added inline comments. INLINE COMMENTS > fdreceiver.cpp:62 > } > +::unlink(QFile::encodeName(m_path)); > } any particular reason not to use QFile::remove() here as well? REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D10410 To: chinmoyr, #f

D8532: [WIP] Restrict file extractor with Seccomp

ossi added a comment. > But i'm not sure if thats feasible. launching subprocesses is no biggie; e.g., the kprocess test does it. if you're lucky, a few env variables will be sufficient. in the worst case, you'd need to chroot and do bind mounts or something like that, which is of cour

D10273: Create proper SocketAddress

ossi added inline comments. INLINE COMMENTS > sharefd_p.h:61 > +const size_t pathSize = finalPath.size(); > +if (pathSize > 5 && pathSize < sizeof(a.sun_path) - 1) { > #ifdef __linux__ you now have a buffer overflow on linux. you need to split the conditional. REPOSITORY R241

D10273: Create proper SocketAddress

ossi added inline comments. INLINE COMMENTS > fdsender.cpp:27 > { > +SocketAddress addr(path.c_str()); > +if (!addr.address()) { this actually constructs a qbytearray. that should be probably explicit. a better approach would be moving the class' interface to qbytearray (or actually q

D10273: Create proper SocketAddress

ossi added inline comments. INLINE COMMENTS > chinmoyr wrote in sharefd_p.h:60 > I just feel like the job of SocketAddress should be to create the address > structure and not perform any file operation. Removal of the socket file > should be handled by file ioslave or FdReceiver. that's a vali

D10273: Create proper SocketAddress

ossi added inline comments. INLINE COMMENTS > sharefd_p.h:60 > -::strcpy(a.sun_path, finalPath.c_str()); > -::unlink(finalPath.c_str()); > -#endif you still need to address that *somehow* ;) > sharefd_p.h:61 > +if (pathSize > 0 && pathSize < sizeof(a.sun_path) - 1) { > +

D10273: Create proper SocketAddress

ossi added inline comments. INLINE COMMENTS > fdreceiver.h:45 > +QString m_path; > +QSocketNotifier *m_readNotifier; > }; why are you moving the member? it doesn't matter in this case, but generally it's better to have the bigger members first, concentrating in particular on equal size

D9966: [KIO] Fix issues with sharing of file descriptor

ossi added a comment. In https://phabricator.kde.org/D9966#199979, @dfaure wrote: > Thiago: now it's ready for your review ;) if he wants to look past the already pointed out non-atomicity. i for one just refuse to look at this mess any further. https://community.kde.org/Policie

Re: kcrash, fork, and stdout/stderr

On Tue, Dec 05, 2017 at 01:12:30PM +0100, David Faure wrote: > On lundi 4 décembre 2017 17:04:25 CET Thiago Macieira wrote: > > forking inside a signal handler is a bad idea because it may deadlock. If > > the crash happens while glibc holds some mutexes relating to > > pthread_atfork, then you'll

D8532: [WIP] Restrict file extractor with Seccomp

ossi added a comment. you *really* should use a whitelist. it's ok if that breaks some 3rdparty extractor; you'll get a bug report which you can properly evaluate. you could go totally overboard and assign fine-grained syscall capabilities to individual extractors, but i can't really think

D8254: Make file ioslave backend on removeables are user-frendly on linux

ossi added inline comments. INLINE COMMENTS > file_unix.cpp:241 > QFile dest_file(dest); > -if (!dest_file.open(QIODevice::Truncate | QIODevice::WriteOnly)) { > +int dest_fd = ::open(dest.toStdString().c_str(), O_CREAT | O_TRUNC | > (is_removeable? O_SYNC : 0) | O_WRONLY); > +if

Re: Review Request 130084: Add a pair of flags forcing fsync during copy loop

ggable ... src/ioslaves/file/file_unix.cpp (line 324) <https://git.reviewboard.kde.org/r/130084/#comment68665> i'd use some parens here to make it more readable. - Oswald Buddenhagen On April 17, 2017, 1:27 p.m., KJ Tsanaktsidis wrote: > > ---

Re: Review Request 130084: Add a pair of flags forcing fsync during copy loop

e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/130084/ > --- > > (Updated April 15, 2017, 10:28 a.m.) > > > Review request for KDE Frameworks, Oswald Buddenhagen and Thiago Macieira. > > > Repository: kio > > > Description > --- > > When cop

D4975: Prevent misdetection of EOF on Linux

ossi added a comment. yeah, but without knowing the root cause, your fix may be just wrong, or at least the comment/commit message may be misleading. also, https://community.kde.org/Policies/Commit_Policy#Don.27t_commit_code_you_don.27t_understand REPOSITORY R291 KPty REVISION DETAIL

D4975: Prevent misdetection of EOF on Linux

ossi added a comment. first try googling it; maybe there is even a response on stackoverflow (as so often happens). then try posting it yourself to stackoverflow. dunno whether such questions are welcome on the linux-kernel list, but if not, there might be other lists where it would fit.

D4975: Prevent misdetection of EOF on Linux

ossi added a comment. please verify that this is in fact a kernel bug (include ml references), not something that should be expected for some weird reasons (compare the solaris path). if this is a bug, does it actually affect released versions which are not being "upgraded away" on short n

Re: Review Request 129122: Try to use ulog-helper if utempter does not exist

omit the other commit for now. - Oswald Buddenhagen On Oct. 20, 2016, 7:52 a.m., Tobias Berner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.

Re: Review Request 129197: Fix tests on FreeBSD

tps://git.reviewboard.kde.org/r/129197/#comment67266> just use QStandardPaths::findExecutable() anyway, that's for a separate commit. - Oswald Buddenhagen On Oct. 16, 2016, 11:44 a.m., Gleb Popov wrote: > > --- > This

Re: Review Request 129197: Fix tests on FreeBSD

- > > (Updated Oct. 16, 2016, 11:44 a.m.) > > > Review request for KDE Frameworks, Adriaan de Groot, Tobias Berner, Oswald > Buddenhagen, and Martin Tobias Holmedahl Sandsmark. > > > Repository: kpty > > > Description > --- &g

Re: Review Request 129122: Try to use ulog-helper if utempter does not exist

;) cmake/FindUTEMPTER.cmake (line 47) <https://git.reviewboard.kde.org/r/129122/#comment67264> just noticed: indentation wrong. - Oswald Buddenhagen On Oct. 19, 2016, 7:51 p.m., Tobias Berner wrote: > > --- > This is a

Re: Review Request 129122: Try to use ulog-helper if utempter does not exist

ne 78) <https://git.reviewboard.kde.org/r/129122/#comment67254> no space after the ! or just use ifndef - Oswald Buddenhagen On Oct. 13, 2016, 2:08 p.m., Tobias Berner wrote: > > --- > This is an automatically

Re: Review Request 128790: Call utempter manually

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128790/#review99696 --- Ship it! Ship It! - Oswald Buddenhagen On Oct. 1, 2016

Re: Review Request 128790: Call utempter manually

> On Sept. 23, 2016, 5:17 a.m., Oswald Buddenhagen wrote: > > cmake/FindUTEMPTER.cmake, line 53 > > <https://git.reviewboard.kde.org/r/128790/diff/5/?file=476595#file476595line53> > > > > i suppose it would make sense to keep it, but with the executa

Re: Review Request 128790: Call utempter manually

/FindUTEMPTER.cmake <https://git.reviewboard.kde.org/r/128790/#comment66914> i suppose it would make sense to keep it, but with the executable. - Oswald Buddenhagen On Sept. 9, 2016, 4:50 a.m., Martin Tobias Holmedahl Sandsmark wrote: > > --

Re: Review Request 128790: Call utempter manually

e 28) <https://git.reviewboard.kde.org/r/128790/#comment66624> dead include - Oswald Buddenhagen On Sept. 4, 2016, 10:20 a.m., Martin Tobias Holmedahl Sandsmark wrote: > > --- > This is an automatically generated e-mai

Re: Review Request 128790: Call utempter manually

> On Aug. 28, 2016, 3:24 p.m., Oswald Buddenhagen wrote: > > src/kpty.cpp, line 165 > > <https://git.reviewboard.kde.org/r/128790/diff/2/?file=475587#file475587line165> > > > > hmpf, i'd really prefer this to be a configure/cmake thing rather than >

Re: Review Request 128790: Call utempter manually

528) <https://git.reviewboard.kde.org/r/128790/#comment66495> please add a comment noting that you're reproducing libutempter x.y behavior on platform foo. just in case there are still incompatible versions around. - Oswald Buddenhagen On Aug. 28, 2016, 2:58 p.m., Martin Tobias Hol

Re: Review Request 128790: Remove usage of utempter

> On Aug. 28, 2016, 1:41 p.m., Oswald Buddenhagen wrote: > > uhm, and why do you think utempter is the preferred choice? > > last time i checked, nobody was shipping konsole setgid utmp. with kdeinit, > > that's not even an option. > > > > there are

Re: Review Request 128790: Remove usage of utempter

ling altogether, as it's been mostly superseded, first by consolekit, and now logind. however, respective bindings would have to be actually implemented, and i have no clue how things are supposed to be done. just some dbus calls? -- what about non-linux systems? - Oswald Buddenhagen On Aug

Re: kptyprocesstest is still flaky

On Sun, May 22, 2016 at 03:59:59PM +0200, David Faure wrote: > The CI for KF5 is now all green except for kpty which is still flaky. > > https://build.kde.org/view/Frameworks%20kf5-qt5/job/kpty%20master%20stable-kf5-qt5/42/PLATFORM=Linux,compiler=gcc/testReport/junit/%28root%29/TestSuite/kptyproce

Re: Review Request 123811: Use tcgetattr & tcsetattr if available

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123811/#review80499 --- Ship it! Ship It! - Oswald Buddenhagen On May 16, 2015, 1

Re: Review Request 123811: Use tcgetattr & tcsetattr if available

g/r/123811/#comment55181> there is no point in leaving that path in, and it makes the code less readable. - Oswald Buddenhagen On May 16, 2015, 9:56 a.m., Pino Toscano wrote: > > --- > This is an automatically generated e

Re: Review Request 118692: Fix reading of entries for language/country combinations

ct puts unreasonable demands on the population of the entries. i suggest you talk to albert. - Oswald Buddenhagen On June 16, 2014, 8:26 a.m., Martin Gräßlin wrote: > > --- > This is an automatically generated e-m

Re: Review Request 118692: Fix reading of entries for language/country combinations

hink. though i seem to remember something like es_ES@valencia. src/core/kconfigini.cpp <https://git.reviewboard.kde.org/r/118692/#comment41821> this is slow. use indexOf on locale. - Oswald Buddenhagen On June 14,

Re: Review Request 116699: No longer use pid_t or Q_PID

::pid()). it simply returns a somewhat wasteful qint64. you may want to follow the same practice. windows does of course not have a native getpid() function, but it is easy enough to replicate. - Oswald Buddenhagen On March 12, 2014, 7:36 a.m., Alexander Richardson wrote

Re: KIO progress towards tier 1 framework?

On Wed, Jul 10, 2013 at 10:01:15AM +0200, Kevin Ottens wrote: > On Tuesday 02 July 2013 23:18:59 Stephen Kelly wrote: > > Note that QtScript can't be loaded in a plugin, so using it in KTranscript > > relies on that changing anyway: > > > > https://bugs.webkit.org/show_bug.cgi?id=38193 > > Is it

Re: Review Request 109667: Make some KConfig classes return a bool when saving

, but generally the patch looks correct. tier2/kconfig/autotests/kconfigguitest.cpp <http://git.reviewboard.kde.org/r/109667/#comment25551> you should wrap the line as you change the code. - Oswald Buddenhagen On June 22, 2013, 9:31 a.m., Albert Astal

Re: Finalized proposal for i18n in KF5, second iteration

On Mon, May 20, 2013 at 12:08:36AM +0200, Chusslove Illich wrote: > > [: Oswald Buddenhagen, 2013-04-06 :] > > the .h file postprocessing in \subsubsection link_ui is a gross hack, and > > i don't see any plausible reason why this should not be fixed in uic > > instea

Re: please make it easier to hack on frameworks

On Mon, May 13, 2013 at 09:43:56PM +0200, Alexander Neundorf wrote: > I did: > $ git pull > $ git submodule update > $ rm -rf builddir (as recommended by Ossi) > $ mkdir build > $ cd build > $ ../configure -prefix $KF5 -opensource... > with qmake, it is never a particularly good idea to put the b

Re: KF5: Let's get KDEUI down

On Tue, May 07, 2013 at 07:13:03PM +0200, Kevin Ottens wrote: > Hoping to see you there! Let's bring KDEUI down. > according to the wiki, the KAddons have the requirement to depend only on Q, but not KAddons. to me this seems totally unreasonable - see for example KImageCache. it's almost as if Qt

Re: please make it easier to hack on frameworks

d sometimes goes a long way. and ccache is cool for testing patches anyway (apply => revert => no-op rebuild). /* Copyright (C) 2013 Oswald Buddenhagen Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met:

Re: please make it easier to hack on frameworks

On Sun, May 05, 2013 at 09:32:49AM +0200, Kevin Ottens wrote: > On Saturday 04 May 2013 22:16:51 Alexander Neundorf wrote: > > On Saturday 04 May 2013, Kevin Ottens wrote: > > > On Saturday 04 May 2013 20:59:32 Alexander Neundorf wrote: > > > > Do I have to run configure again, or is qmake in the m

Re: Review Request 110264: Change uses of QCoreApplication::translate() with no context to tr() in tier1 frameworks.

de.org/r/110264/#comment23842> you could add the missing spaces around the equal signs as you are changing this code ... - Oswald Buddenhagen On May 3, 2013, 8:35 p.m., George Goldberg wrote: > > --- > This is an automatically genera

Re: Review Request 110264: Change uses of QCoreApplication::translate() with no context to QObject::tr() in tier1 frameworks.

ttp://git.reviewboard.kde.org/r/110264/#comment23764> *never* use QObject::tr() (unless you are qobject.cpp, obviously). http://qt-project.org/doc/qt-5.0/qtdoc/internationalization.html#translating-non-qt-classes - Oswald Buddenhagen On May 1, 2013, 9:37 p.m., George Goldberg

Re: Finalized proposal for i18n in KF5, second iteration

On Tue, Apr 02, 2013 at 03:46:52PM +0200, Chusslove Illich wrote: > After considering comments from the previous iteration: > > http://mail.kde.org/pipermail/kde-frameworks-devel/2012-December/001358.html > > here is the updated version: > just a quick scan of the diffs from me: the .h file p

Re: Review Request 109667: Make some KConfig classes return a bool when saving

ttp://git.reviewboard.kde.org/r/109667/#comment22286> this actually has a good chance of failing on windows. you may need to restore the write permissions first. - Oswald Buddenhagen On March 26, 2013, 11:25 p.m., Albert Astals Cid

Re: Review Request 109667: Make some KConfig classes return a bool when saving

> On March 23, 2013, 9:56 p.m., Oswald Buddenhagen wrote: > > tier2/kconfig/src/core/kconfig.cpp, line 440 > > <http://git.reviewboard.kde.org/r/109667/diff/2/?file=121088#file121088line440> > > > > well, it appears to fit the pre-existing logic. any parti

Re: Review Request 109551: port KPtyProcess to QProcess

nation how that would correlate with your hangs. - Oswald Buddenhagen On March 18, 2013, 7:54 p.m., Martin Tobias Holmedahl Sandsmark wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.review

Re: Review Request 109667: Make some KConfig classes return a bool when saving

667/#comment22159> typo tier2/kconfig/src/core/kconfig.cpp <http://git.reviewboard.kde.org/r/109667/#comment22160> well, it appears to fit the pre-existing logic. any particular reason why you are not sure? - Oswald Buddenhagen On March 23, 2013, 3:47 p.m., Albert Asta

Re: Review Request 109551: port KPtyProcess to QProcess

> On March 18, 2013, 10:04 p.m., Oswald Buddenhagen wrote: > > kpty/tests/kptyprocesstest.cpp, line 193 > > <http://git.reviewboard.kde.org/r/109551/diff/2/?file=120310#file120310line193> > > > > i don't think eating the sleep is a good idea. i

Re: Review Request 109551: port KPtyProcess to QProcess

y/tests/kptyprocesstest.cpp <http://git.reviewboard.kde.org/r/109551/#comment21998> the -c needs to be a separate argument. the quotes, backslashes and attempt at a newline are all garbage. - Oswald Buddenhagen On March 18, 2013, 7:54 p.m., Martin

Re: Review Request 109551: port KPtyProcess to QProcess

ttp://git.reviewboard.kde.org/r/109551/#comment21968> why should they? you already have the solution in the previous hunk. - Oswald Buddenhagen On March 17, 2013, 4:44 p.m., Martin Tobias Holmedahl Sandsmark wrote: > > --- > This is an automatica

Re: libexec (Re: "kde5" or "kf5" ?)

On Sat, Feb 23, 2013 at 11:25:59PM +0100, David Faure wrote: > On Saturday 23 February 2013 19:05:34 Alexander Neundorf wrote: > > At configure/cmake time, we can configure e.g. the data install dir, the > > lib install dir and the install prefix into the library. > > At that point, we can check w

Re: Review Request 108389: Respect the HOME environment variable

org/r/108389/ > --- > > (Updated Jan. 19, 2013, 10:52 a.m.) > > > Review request for KDE Frameworks and Oswald Buddenhagen. > > > Description > --- > > This turns KUser::homeDir (and thus KShell::tildeExpand) closer to > QDir::homePath, which only consults th

Re: Review Request 108389: Respect the HOME environment variable

cerned about the extra getuid calls, you could pass this in an (optional) parameter. - Oswald Buddenhagen On Jan. 19, 2013, 10:52 a.m., Jon Severinsson wrote: > > --- > This is an automatically generated e-mail

Re: Review Request 108385: Reduce risk of timeout and race condition in KPtyProcessTest

> On Jan. 13, 2013, 6:45 p.m., Oswald Buddenhagen wrote: > > and why exactly do you sleep instead of looping with waitforreadyread? > > Jon Severinsson wrote: > Because that would be an (almost) busy-loop (there are already *some* > data, so waitForReadyRead could retu

  1   2   >