Skip to content

Commit 5fb04ba

Browse files
wdc: use libnvme's nvme_uuid_find() for UUID lookup
Replace the internal implementation for finding specific UUIDs with the `nvme_uuid_find()` function provided by libnvme. This change simplifies the code and improves maintainability by leveraging a well-tested utility function from the library. While the internal implementation might have offered slightly better performance as we looped only once, the clarity and reduced code duplication are preferred. Signed-off-by: Tomas Winkler <[email protected]>
1 parent 7e3424f commit 5fb04ba

3 files changed

Lines changed: 40 additions & 95 deletions

File tree

plugins/wdc/wdc-nvme.c

Lines changed: 40 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -524,24 +524,16 @@ enum NVME_FEATURE_IDENTIFIERS {
524524
};
525525

526526
/* WDC UUID value */
527-
const uint8_t WDC_UUID[] = {
528-
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
529-
0x00, 0x00, 0x00, 0x00, 0x00, 0x2d, 0xb9, 0x8c, 0x52, 0x0c, 0x4c,
530-
0x5a, 0x15, 0xab, 0xe6, 0x33, 0x29, 0x9a, 0x70, 0xdf, 0xd0
527+
static const __u8 WDC_UUID[NVME_UUID_LEN] = {
528+
0x2d, 0xb9, 0x8c, 0x52, 0x0c, 0x4c, 0x5a, 0x15,
529+
0xab, 0xe6, 0x33, 0x29, 0x9a, 0x70, 0xdf, 0xd0
531530
};
532531

533-
/* WDC_UUID value for SN640_3 devices */
534-
const uint8_t WDC_UUID_SN640_3[] = {
535-
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
536-
0x00, 0x00, 0x00, 0x00, 0x00, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11,
537-
0x11, 0x11, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22
538-
};
539532

540-
/* UUID field with value of 0 indicates end of UUID List*/
541-
const uint8_t UUID_END[] = {
542-
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
543-
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
544-
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
533+
/* WDC_UUID value for SN640_3 devices */
534+
static const __u8 WDC_UUID_SN640_3[NVME_UUID_LEN] = {
535+
0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11,
536+
0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22
545537
};
546538

547539
enum WDC_DRIVE_ESSENTIAL_TYPE {
@@ -1585,7 +1577,6 @@ static bool wdc_is_zn350(__u32 device_id)
15851577
return (device_id == WDC_NVME_ZN350_DEV_ID ||
15861578
device_id == WDC_NVME_ZN350_DEV_ID_1);
15871579
}
1588-
15891580
static bool needs_c2_log_page_check(__u32 device_id)
15901581
{
15911582
if ((wdc_is_sn640(device_id)) ||
@@ -2663,8 +2654,7 @@ static bool get_dev_mgment_data(nvme_root_t r, struct nvme_dev *dev,
26632654
bool found = false;
26642655
*data = NULL;
26652656
__u32 device_id = 0, vendor_id = 0;
2666-
bool uuid_present = false;
2667-
int index = 0, uuid_index = 0;
2657+
int uuid_index = 0;
26682658
struct nvme_id_uuid_list uuid_list;
26692659

26702660
/* The wdc_get_pci_ids function could fail when drives are connected
@@ -2675,45 +2665,27 @@ static bool get_dev_mgment_data(nvme_root_t r, struct nvme_dev *dev,
26752665
*/
26762666
wdc_get_pci_ids(r, dev, &device_id, &vendor_id);
26772667

2678-
typedef struct nvme_id_uuid_list_entry *uuid_list_entry;
2679-
26802668
memset(&uuid_list, 0, sizeof(struct nvme_id_uuid_list));
26812669
if (wdc_CheckUuidListSupport(dev, &uuid_list)) {
2682-
uuid_list_entry uuid_list_entry_ptr = (uuid_list_entry)&uuid_list.entry[0];
2683-
2684-
while (index <= NVME_ID_UUID_LIST_MAX &&
2685-
!wdc_UuidEqual(uuid_list_entry_ptr, (uuid_list_entry)UUID_END)) {
2686-
2687-
if (wdc_UuidEqual(uuid_list_entry_ptr,
2688-
(uuid_list_entry)WDC_UUID)) {
2689-
uuid_present = true;
2690-
break;
2691-
} else if (wdc_UuidEqual(uuid_list_entry_ptr,
2692-
(uuid_list_entry)WDC_UUID_SN640_3) &&
2693-
wdc_is_sn640_3(device_id)) {
2694-
uuid_present = true;
2695-
break;
2670+
uuid_index = nvme_uuid_find(&uuid_list, WDC_UUID);
2671+
if (uuid_index > 0) {
2672+
found = get_dev_mgmt_log_page_data(dev, data, uuid_index);
2673+
} else {
2674+
uuid_index = nvme_uuid_find(&uuid_list, WDC_UUID_SN640_3);
2675+
if (uuid_index > 0 && wdc_is_sn640_3(device_id)) {
2676+
found = get_dev_mgmt_log_page_data(dev, data, uuid_index);
26962677
}
2697-
index++;
2698-
uuid_list_entry_ptr = (uuid_list_entry)&uuid_list.entry[index];
26992678
}
2700-
if (uuid_present)
2701-
uuid_index = index + 1;
27022679
}
27032680

2704-
if (uuid_present) {
2705-
/* use the uuid index found above */
2706-
found = get_dev_mgmt_log_page_data(dev, data, uuid_index);
2707-
} else {
2708-
if (!uuid_index && needs_c2_log_page_check(device_id)) {
2709-
/* In certain devices that don't support UUID lists, there are multiple
2710-
* definitions of the C2 logpage. In those cases, the code
2711-
* needs to try two UUID indexes and use an identification algorithm
2712-
* to determine which is returning the correct log page data.
2713-
*/
2714-
uuid_index = 1;
2715-
}
2681+
if (!found && needs_c2_log_page_check(device_id)) {
2682+
/* In certain devices that don't support UUID lists, there are multiple
2683+
* definitions of the C2 logpage. In those cases, the code
2684+
* needs to try two UUID indexes and use an identification algorithm
2685+
* to determine which is returning the correct log page data.
2686+
*/
27162687

2688+
uuid_index = 1;
27172689
found = get_dev_mgmt_log_page_data(dev, data, uuid_index);
27182690

27192691
if (!found) {
@@ -2737,8 +2709,7 @@ static bool get_dev_mgment_cbs_data(nvme_root_t r, struct nvme_dev *dev,
27372709
__u8 lid = 0;
27382710
*cbs_data = NULL;
27392711
__u32 device_id = 0, vendor_id = 0;
2740-
bool uuid_present = false;
2741-
int index = 0, uuid_index = 0;
2712+
int uuid_index = 0;
27422713
struct nvme_id_uuid_list uuid_list;
27432714

27442715
/* The wdc_get_pci_ids function could fail when drives are connected
@@ -2751,50 +2722,30 @@ static bool get_dev_mgment_cbs_data(nvme_root_t r, struct nvme_dev *dev,
27512722

27522723
lid = WDC_NVME_GET_DEV_MGMNT_LOG_PAGE_ID;
27532724

2754-
typedef struct nvme_id_uuid_list_entry *uuid_list_entry;
2755-
27562725
memset(&uuid_list, 0, sizeof(struct nvme_id_uuid_list));
2757-
if (wdc_CheckUuidListSupport(dev, &uuid_list)) {
2758-
uuid_list_entry uuid_list_entry_ptr = (uuid_list_entry)&uuid_list.entry[0];
27592726

2760-
while (index <= NVME_ID_UUID_LIST_MAX &&
2761-
!wdc_UuidEqual(uuid_list_entry_ptr, (uuid_list_entry)UUID_END)) {
2762-
2763-
if (wdc_UuidEqual(uuid_list_entry_ptr,
2764-
(uuid_list_entry)WDC_UUID)) {
2765-
uuid_present = true;
2766-
break;
2767-
} else if (wdc_UuidEqual(uuid_list_entry_ptr,
2768-
(uuid_list_entry)WDC_UUID_SN640_3) &&
2769-
wdc_is_sn640_3(device_id)) {
2770-
uuid_present = true;
2771-
break;
2727+
if (wdc_CheckUuidListSupport(dev, &uuid_list)) {
2728+
uuid_index = nvme_uuid_find(&uuid_list, WDC_UUID);
2729+
if (uuid_index > 0) {
2730+
found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, log_id,
2731+
uuid_index);
2732+
} else {
2733+
uuid_index = nvme_uuid_find(&uuid_list, WDC_UUID_SN640_3);
2734+
if (uuid_index > 0 && wdc_is_sn640_3(device_id)) {
2735+
found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, log_id,
2736+
uuid_index);
27722737
}
2773-
index++;
2774-
uuid_list_entry_ptr = (uuid_list_entry)&uuid_list.entry[index];
27752738
}
2776-
if (uuid_present)
2777-
uuid_index = index + 1;
27782739
}
27792740

2780-
if (uuid_present) {
2781-
/* use the uuid index found above */
2782-
found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, log_id, uuid_index);
2783-
} else if (wdc_is_zn350(device_id)) {
2784-
uuid_index = 0;
2785-
found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, log_id, uuid_index);
2786-
} else {
2787-
if (!uuid_index && needs_c2_log_page_check(device_id)) {
2788-
/* In certain devices that don't support UUID lists, there are multiple
2789-
* definitions of the C2 logpage. In those cases, the code
2790-
* needs to try two UUID indexes and use an identification algorithm
2791-
* to determine which is returning the correct log page data.
2792-
*/
2793-
uuid_ix = 1;
2794-
}
2795-
2741+
if (!found && needs_c2_log_page_check(device_id)) {
2742+
/* In certain devices that don't support UUID lists, there are multiple
2743+
* definitions of the C2 logpage. In those cases, the code
2744+
* needs to try two UUID indexes and use an identification algorithm
2745+
* to determine which is returning the correct log page data.
2746+
*/
2747+
uuid_ix = 1;
27962748
found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, log_id, uuid_ix);
2797-
27982749
if (!found) {
27992750
/* not found with uuid = 1 try with uuid = 0 */
28002751
uuid_ix = 0;

plugins/wdc/wdc-utils.c

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,3 @@ bool wdc_CheckUuidListSupport(struct nvme_dev *dev, struct nvme_id_uuid_list *uu
189189

190190
return false;
191191
}
192-
193-
bool wdc_UuidEqual(struct nvme_id_uuid_list_entry *entry1, struct nvme_id_uuid_list_entry *entry2)
194-
{
195-
return !memcmp(entry1->uuid, entry2->uuid, NVME_UUID_LEN);
196-
}

plugins/wdc/wdc-utils.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,4 +78,3 @@ int wdc_UtilsCreateDir(char *path);
7878
int wdc_WriteToFile(char *fileName, char *buffer, unsigned int bufferLen);
7979
void wdc_StrFormat(char *formatter, size_t fmt_sz, char *tofmt, size_t tofmtsz);
8080
bool wdc_CheckUuidListSupport(struct nvme_dev *dev, struct nvme_id_uuid_list *uuid_list);
81-
bool wdc_UuidEqual(struct nvme_id_uuid_list_entry *entry1, struct nvme_id_uuid_list_entry *entry2);

0 commit comments

Comments
 (0)