intel / xpumanager

MIT License
87 stars 18 forks source link

Multiple copies for parsing "uevent" sysfs file #80

Open eero-t opened 4 months ago

eero-t commented 4 months ago

Noticed multiple places having almost identical code for parsing sysfs uevent file:

$ git grep printf.*/uevent
cli/src/local_functions.cpp:            snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/uevent", pdirent->d_name);
core/src/device/gpu/gpu_device_stub.cpp:        len = snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/uevent",
core/src/device/gpu/gpu_device_stub.cpp:        len = snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/uevent",
core/src/diagnostic/diagnostic_manager.cpp:        len = snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/uevent",
core/src/diagnostic/precheck.cpp:                    snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/uevent", pdirent->d_name);

I would suggest consolidating all of them to a single helper function that is provided e.g. the PCI BDF string that should be matched.

testdig commented 4 months ago

@eero-t,

Thanks for the suggestion. We would refactor code in the next sprints. One exception: CLI and core would not share helper functions because the only interface between CLI and core is XPUM API in the current design.