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

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,

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,

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

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

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

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

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 > + >

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

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

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

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

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

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 >

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

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

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,

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,

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

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

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(),

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(); > +return

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,

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

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

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,

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

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

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

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

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

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.

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); > +

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

e ... 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

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

isit: > 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 copying a large-ish fil

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

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

to 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 is a

Re: Review Request 129197: Fix tests on FreeBSD

16, 11:44 a.m.) > > > Review request for KDE Frameworks, Adriaan de Groot, Tobias Berner, Oswald > Buddenhagen, and Martin Tobias Holmedahl Sandsmark. > > > Repository: kpty > > > Description > --- > > Apparently, KPtyDevice can't be operated

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

) <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 g

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

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

<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 Holmedahl San

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. > > > > ther

Re: Review Request 128790: Remove usage of utempter

, 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. 28, 2016, 1:13 p.m

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. > >

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

/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-mail. To reply, visit

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

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-mail. To reply, visit: https

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

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, 2014, 8:24 a.m., Martin Gräßlin wrote

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: 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 Astals Cid wrote

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 instead. I agree, but I saw it done like

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... as given in the wiki with qmake, it is never a particularly good idea

Re: please make it easier to hack on frameworks

is cool for testing patches anyway (apply = revert = no-op rebuild). /* Copyright (C) 2013 Oswald Buddenhagen o...@kde.org Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met: 1. Redistributions of source

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

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 generated e-mail. To reply, visit: http

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

Re: Review Request 109551: port KPtyProcess to QProcess

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.reviewboard.kde.org/r/109551

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

/#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 Astals Cid wrote

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'm sure i added it for a reason (in a previous life

Re: Review Request 109551: port KPtyProcess to QProcess

/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 automatically generated e-mail

Re: Review Request 109551: port KPtyProcess to QProcess

://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 Tobias Holmedahl Sandsmark wrote

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 return before the timeout

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

with waitforreadyread? - Oswald Buddenhagen On Jan. 13, 2013, 1:03 p.m., Jon Severinsson wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108385

Re: Finalized proposal for changes to i18n in KF5

On Tue, Jan 08, 2013 at 03:05:25PM +0100, Chusslove Illich wrote: [: Chusslove Illich :] I'm not opposed to some additional bureaucracy in order to make the framework more accessible to potential users. [...] [: Oswald Buddenhagen :] [...] i'd hate it to see my thoroughly engineered

Re: Finalized proposal for changes to i18n in KF5

On Sat, Jan 05, 2013 at 06:38:58PM +0100, Chusslove Illich wrote: [: Oswald Buddenhagen :] of course, it would be even better if you strived for submission to qt- project, if at all realistic (for now probably an add-on, but definitely under cla). otherwise you'll see the same effect every

Re: Review Request: port Sonnet to QSettings

, way more advanced and maintained settings. If the sole goal of this endavor is to remove the KConfig dependency than this ought to be done by either having an interface that can be implemented through KConfig or by passing values as QVariant maps or hashes. Oswald Buddenhagen wrote

Re: Review Request: Port some uses of KProcess to QProcess

On Sat, Sep 22, 2012 at 12:42:26PM +0200, David Faure wrote: On Thursday 20 September 2012 22:02:03 Oswald Buddenhagen wrote: yes. a rather suboptimal replacement. before you ask, search the core-devel archive shortly before the time the new kprocess was added. I can't do *everything

Re: Review Request: Port some uses of KProcess to QProcess

On Sept. 19, 2012, 5:11 p.m., Oswald Buddenhagen wrote: every single change of this patch is a regression David Faure wrote: Very encouraging, as always. Care to give us more details? the reasons for kprocess' existence didn't go away, so every change away from it is by definition

Re: Review Request: Port some uses of KUrl to QUrl

/106501/#comment15262 this whole hunk doesn't belong into this patch at all. one part of it belongs into the kprocess patch, the other (the kdesuExe part) is a commit of its own. - Oswald Buddenhagen On Sept. 19, 2012, 10:19 p.m., Kevin Funk wrote

Re: Review Request: Port some uses of KProcess to QProcess

On Sept. 19, 2012, 5:11 p.m., Oswald Buddenhagen wrote: every single change of this patch is a regression David Faure wrote: Very encouraging, as always. Care to give us more details? Oswald Buddenhagen wrote: the reasons for kprocess' existence didn't go away, so every change

Re: Review Request: Port some uses of KProcess to QProcess

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106500/#review19174 --- every single change of this patch is a regression - Oswald

  1   2   >