search

EstebanBorai / network-interface

Retrieve system's Network Interfaces on Linux, macOS and Windows on a standardized manner
https://crates.io/crates/network-interface
Apache License 2.0
62 stars 27 forks source link

Fix/windows memory leaks #54

Closed scanban closed 7 months ago

scanban commented 7 months ago

Removed 2 memory leaks in windows related code, added FFI output buffer management wit RAII. Removed not needed memory allocation(s)

PS. ohh, looks like @njeisecke already added same ;) Feel free to drop mine if not needed

njeisecke commented 7 months ago

That's funny. I think your generic FFIAlloc is better than my AdapterAddressBufferGuard. However I prefer my alloc retry logic and error handling, feels more like proper rust to me ;-)

njeisecke commented 7 months ago

I think 0b2b6b76fd9b2a9ff35f764347109ad64f636d4a contains changes not actually related to the leak (probably for the better, but this should be a separate commit).

scanban commented 7 months ago

I think 0b2b6b7 contains changes not actually related to the leak (probably for the better, but this should be a separate commit).

it is, plus some simplifications/style of the code. Next should be creating of network interfaces based on interface index with multiple addresses to be on par with the Linux results.

Btw, what tool you are using on Win for detecting memory leaks ? Windows is not my primary development platform.

EstebanBorai commented 7 months ago

Hi @scanban! I can see you are also participanting in the PR opened by @njeisecke. Would you like to focus in such PR? So we all participate in the same effort?

scanban commented 7 months ago

Hi @scanban! I can see you are also participanting in the PR opened by @njeisecke. Would you like to focus in such PR? So we all participate in the same effort?

Hello @EstebanBorai, yea sure. I think that @njeisecke could get some bits from my changes if he decide to do that. And my PR then could be dropped.

njeisecke commented 7 months ago

Yeah, I would use the generic RAII wrapper and restore the retry counter. @scanban, would you then do a follow-up with your other changes?

njeisecke commented 7 months ago

Ok, #53 now also contains the @scanban changes relevant to the leak fix.

scanban commented 7 months ago

Yeah, I would use the generic RAII wrapper and restore the retry counter. @scanban, would you then do a follow-up with your other changes?

yea, sure