Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

General refactor of PcapLiveDeviceList and PcapRemoteDeviceList. #1431

Open
Dimi1010 opened this issue Jun 5, 2024 · 0 comments
Open

General refactor of PcapLiveDeviceList and PcapRemoteDeviceList. #1431

Dimi1010 opened this issue Jun 5, 2024 · 0 comments
Assignees

Comments

@Dimi1010
Copy link
Collaborator

Dimi1010 commented Jun 5, 2024

This is an issue to track progress that will be done over several PRs.

Task overview:

    • Refactor PcapLiveDeviceList to use smart pointers internally for memory management.
    • Refactor PcapRemoteDevice to keep a shared reference of RemoteAuthentication with PcapRemoteDeviceList via std::shared_ptr as it currently uses a raw pointer to share authentication with the device list.
    • Unify the public "getDevice" API of both lists using smart pointers.
    • Refactor PcapRemoteDeviceList's factories to return unique_ptr instead of raw pointers.
    • Centralize code calling Libpcap/Winpcap/Npcap to fetch devices into a utility header.

1. Use smart pointers for internal memory management in PcapLiveDeviceList

The current implementation uses a vector of raw pointers and manually handles new/delete calls. The move towards smart pointers will automate the need for manual memory management.

2. Use smart pointers for internal memory management in PcapRemoteDeviceList

The current implementation uses a vector of raw pointers and manually handles new/delete calls. The move towards smart pointers will automate the need for manual memory management.

3. Use shared_ptr to store RemoteAuthentication in PcapRemoteDeviceList and PcapRemoteDevice.

The current implementation uses a raw pointer to RemoteAuthentication shared between PcapRemoteDeviceList and its PcapRemoteDevices. The move towards smart pointers will relieve the need for PcapRemoteDeviceList to need to manage the lifetime of the authentication object manually, and will prevent potentially deallocating it while another PcapRemoteDevice is using it.

4. Unifying the public API for fetching single devices.

Currently the public APIs of PcapLiveDeviceList and PcapRemoteDeviceList use similar but not entirely the same syntax for getter functions.

  • PcapLiveDeviceList uses getPcapLiveDeviceBy* returning a raw pointer to PcapLiveDevice
  • PcapRemoteDeviceList uses getPcapRemoteDeviceBy* returning a raw pointer to PcapRemoteDevice

The proposed task is to unify the API of both lists so they can be used interchangeably. This may also allow centralizing much of the getter functionality under a base class as both lists generally keep their devices in a vector of pointers. Additional list specific class members can stay in their respective classes.

Working draft syntax for the methods is as follows:

  • Function syntax getPcapDeviceBy* or getDeviceBy*
  • Return type shared_ptr<PcapLiveDevice> as PcapRemoteDevice is a virtual subclass of PcapLiveDevice.

5. Unify the public API for iterating over all devices.

Currently PcapLiveDeviceList and PcapRemoteDeviceList provide different APIs for iterating over all devices in the list.

  • PcapLiveDeviceList uses the method getPcapLiveDevicesList returning a vector by reference
  • PcapRemoteDeviceList implements container style begin/end methods.

The proposed change is to unify the APIs, providing common methods between the two classes.
The proposed API is as follows:

  • A method getAllPcapDevices returning vector<shared_ptr<PcapLiveDevice>>.
    NB: PcapRemoteDeviceList may implement additional getAllPcapRemoteDevices that returns vector<shared_ptr<PcapRemoteDevice>>
  • A container style begin/end methods. Return signature TBD. (May conflict with point 4's base class vector.)

6. Refactor PcapRemoteDeviceList's factories to return unique_ptr instead of raw pointers

The current implementation of the remote device list's factories returns raw pointers to the device list, relying on the caller to deallocated the list via a delete call. Changing the factory to return a unique_ptr will increase memory safety by automatically releasing the memory when the user is done with the list,

7. Centralize code calling Libpcap/Winpcap/Npcap to fetch devices into a utility header.

Currently there are duplicated calls to fetch all devices in PcapLiveDeviceList and during the clone procedure in PcapLiveDevice
Centralizing that code to its own dedicated utilities header will make it easier to maintain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant