[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #2105 [Vidalia]: Pick up the breakpad integration
#2105: Pick up the breakpad integration
-------------------------+--------------------------------------------------
Reporter: arma | Owner: chiiph
Type: enhancement | Status: assigned
Priority: major | Milestone: Vidalia-0.3.X
Component: Vidalia | Version:
Keywords: | Parent:
Points: | Actualpoints:
-------------------------+--------------------------------------------------
Comment(by chiiph):
Replying to [comment:7 edmanm]:
> So a couple of comments:
>
First of all, remember, that code is just me testing breakpad and such.
> 1) The fact that I did not use Qt API methods for stuff in the exception
handler is 'absolutely' intentional. When the minidump callback is called,
the application is probably in a bad state and so you want to do as little
as necessary in the handler. Calling out to complex, unnecessary
libraries, such as Qt, is not a good idea.
Ok, I'll dig into this issue. It was just the quickest way to make it
Linux friendly and test a couple of things.
Given the current state of code protection I'd bet that Qt's code is still
in good shape in any platform.
>
> 2) You broke crash dump support on Windows systems using wide chars
(unless something has significantly changed in recent versions of
Breakpad).
I was just testing this on Linux, again, this is *not* finished at all.
>
> 3) I don't think this does what you think it does:
>
> {{{
> - target_link_libraries(${vidalia_BIN} ${BREAKPAD_LIBRARIES})
> + target_link_libraries(${vidalia_BIN} ${BREAKPAD_LIBRARY_DIR})
> }}}
>
> You're trying to link a binary to a directory, rather than the actual
libraries (unless you define `BREAKPAD_LIBRARY_DIR` to a specific file
rather than a directory to search for the correct files, but that's
nonsense). See `FindBreakpad.cmake`.
Yes, it does exactly what I want in the particular way I was testing it, I
was using BREAKPAD_LIBRARY_DIR to point to the .a file compiled in the
google-breakpad dir, The find cmake macro didn't worked in my setup, so I
got tired of it and did it "by hand".
>
> 4) I don't understand this:
>
> {{{
> + _configDialog = 0;
> }}}
This is just to make it crash and see if it works as expected.
Again, this wasn't for review. This was just to say "hey, I'm looking at
this, this code is wrong/ugly in several ways, but it's the branch in
which I'll be working on this" :)
When I think it's ready, I'll update the ticket as needs_review.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/2105#comment:8>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs