search

rust-minidump / minidump-writer

Rust rewrite of breakpad's minidump_writer
MIT License
68 stars 17 forks source link

Remove the custom HANDLE stream #56

Closed gabrielesvelto closed 2 years ago

Jake-Shadle commented 2 years ago

I'm wondering if we should even have this at all any longer? I only added this because it was present in Breakpad, but Crashpad doesn't include it and giving how much they have over-engineered everything in Crashpad that kind of leads me to believe that this handle stream was actually not worth porting over?

gabrielesvelto commented 2 years ago

I didn't know that code came from Breakpad... we never used it and a quick glance at Microsoft docs reveals that there's a "standard" way of getting that data using the MiniDumpWithHandleData option which should yield a MINIDUMP_HANDLE_DATA_STREAM.

If you're OK with I say we drop it.

Jake-Shadle commented 2 years ago

Cool, I generally feel if neither Chrome nor Firefox use something it's a fairly safe bet that most other projects with (hopefully) fewer crash reports/users are not going to find it useful as well, I think it's fine to just nuke it. If someone complains it's not there we can just add it back behind a feature flag in the future.

gabrielesvelto commented 2 years ago

Thanks! I seem to have flushed out the last test failure, can you merge?