Review Request: KCrash patch: make it able to start DrKonqi on Windows

[George Kiagiadakis]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4761/#review6714
-----------------------------------------------------------


I'm not the best person to review this, since I wrote most of this patch, but here are some comments regarding your additions:


/trunk/KDE/kdelibs/kdeui/util/kcrash.cpp
<http://reviewboard.kde.org/r/4761/#comment6504>

    Just a note on this one: closeAllFDs() compiles fine on windows, but it crashes when it is called. I put it in #ifndef Q_OS_WIN just to avoid this crash, but I'm not sure if it's something that makes sense on windows and should be fixed or if it's safe to ignore it. I would like some comments about this from kde-windows people.



/trunk/KDE/kdelibs/kdeui/util/kcrash.cpp
<http://reviewboard.kde.org/r/4761/#comment6503>

    You forgot to update this. Since you added the --thread option, you need to update this size to 27.



/trunk/KDE/kdelibs/kdeui/util/kcrash.cpp
<http://reviewboard.kde.org/r/4761/#comment6502>

    We have talked about this before. Why not use sprintf() to be consistent with the rest of the code?



/trunk/KDE/kdelibs/kdeui/util/kcrash.cpp
<http://reviewboard.kde.org/r/4761/#comment6505>

    Please fix indentation in this function. Use 4 spaces instead of a tab, like the existing code does.



/trunk/KDE/kdelibs/kdeui/util/kcrash.cpp
<http://reviewboard.kde.org/r/4761/#comment6506>

    Accidental trailing } here. Remove it :)


- George


On 2010-07-27 12:54:01, Ilie Halip wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4761/
> -----------------------------------------------------------
> 
> (Updated 2010-07-27 12:54:01)
> 
> 
> Review request for kde-windows and kdelibs.
> 
> 
> Summary
> -------
> 
> This patch allows starting DrKonqi on Windows. It's based off an older patch by gkiagia which was talked about on the kde-windows mailing list before GSoC started. I also added a piece of shared memory because the debugger I've written needs to get the thread context when the process crashes, and there's a problem if the context is taken after the exception is dispatched by Windows.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/kdeui/util/kcrash.cpp 1155484 
> 
> Diff: http://reviewboard.kde.org/r/4761/diff
> 
> 
> Testing
> -------
> 
> - tested locally with both MSVC2008 and mingw
> 
> 
> Thanks,
> 
> Ilie
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-windows/attachments/20100727/3a20320b/attachment.htm