From: Tokunori Ikegami Date: Sat, 3 Jun 2023 17:38:36 +0000 (+0900) Subject: plugins/micron: Fix micron-nvme.c linux kernel check patch errors and warnings X-Git-Tag: v2.5~43 X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=9b68c153a7b6e0ab781c4e98fac1363489c32550;p=users%2Fsagi%2Fnvme-cli.git plugins/micron: Fix micron-nvme.c linux kernel check patch errors and warnings Note: Still the remaining warning messages are not fixed as same with nvme.c. WARNING: quoted string split across lines + "This performs a selective firmware download, which allows the user to " + "select which firmware binary to update for 9200 devices. This requires " WARNING: quoted string split across lines + "select which firmware binary to update for 9200 devices. This requires " + "a power cycle once the update completes. The options available are:\n\n" Signed-off-by: Tokunori Ikegami --- diff --git a/plugins/micron/micron-nvme.c b/plugins/micron/micron-nvme.c index 73b56b30..0d1a8a68 100644 --- a/plugins/micron/micron-nvme.c +++ b/plugins/micron/micron-nvme.c @@ -21,10 +21,10 @@ #include "micron-nvme.h" /* Supported Vendor specific feature ids */ -#define MICRON_FEATURE_CLEAR_PCI_CORRECTABLE_ERRORS 0xC3 +#define MICRON_FEATURE_CLEAR_PCI_CORRECTABLE_ERRORS 0xC3 #define MICRON_FEATURE_CLEAR_FW_ACTIVATION_HISTORY 0xC1 -#define MICRON_FEATURE_TELEMETRY_CONTROL_OPTION 0xCF -#define MICRON_FEATURE_SMBUS_OPTION 0xD5 +#define MICRON_FEATURE_TELEMETRY_CONTROL_OPTION 0xCF +#define MICRON_FEATURE_SMBUS_OPTION 0xD5 /* Supported Vendor specific log page sizes */ #define C5_log_size (((452 + 16 * 1024) / 4) * 4096) @@ -33,8 +33,8 @@ #define D0_log_size 512 #define FB_log_size 512 #define E1_log_size 256 -#define MaxLogChunk 16 * 1024 -#define CommonChunkSize 16 * 4096 +#define MaxLogChunk (16 * 1024) +#define CommonChunkSize (16 * 4096) #define min(x, y) ((x) > (y) ? (y) : (x)) #define SensorCount 8 @@ -44,10 +44,19 @@ static const char *__version_major = "1"; static const char *__version_minor = "0"; static const char *__version_patch = "14"; -/* supported models of micron plugin; new models should be added at the end +/* + * supported models of micron plugin; new models should be added at the end * before UNKNOWN_MODEL. Make sure M5410 is first in the list ! */ -typedef enum { M5410 = 0, M51AX, M51BX, M51CX, M5407, M5411, UNKNOWN_MODEL } eDriveModel; +enum eDriveModel { + M5410 = 0, + M51AX, + M51BX, + M51CX, + M5407, + M5411, + UNKNOWN_MODEL +}; #define MICRON_VENDOR_ID 0x1344 @@ -58,24 +67,25 @@ static char *fdeviceid2 = "/sys/class/misc/nvme%d/device/device"; static unsigned short vendor_id; static unsigned short device_id; -typedef struct _LogPageHeader_t { +struct LogPageHeader_t { unsigned char numDwordsInLogPageHeaderLo; unsigned char logPageHeaderFormatVersion; unsigned char logPageId; unsigned char numDwordsInLogPageHeaderHi; unsigned int numValidDwordsInPayload; unsigned int numDwordsInEntireLogPage; -} LogPageHeader_t; +}; static void WriteData(__u8 *data, __u32 len, const char *dir, const char *file, const char *msg) { char tempFolder[8192] = { 0 }; FILE *fpOutFile = NULL; + sprintf(tempFolder, "%s/%s", dir, file); - if ((fpOutFile = fopen(tempFolder, "ab+")) != NULL) { - if (fwrite(data, 1, len, fpOutFile) != len) { + fpOutFile = fopen(tempFolder, "ab+"); + if (fpOutFile) { + if (fwrite(data, 1, len, fpOutFile) != len) printf("Failed to write %s data to %s\n", msg, tempFolder); - } fclose(fpOutFile); } else { printf("Failed to open %s file to write %s\n", tempFolder, msg); @@ -103,9 +113,9 @@ static int ReadSysFile(const char *file, unsigned short *id) return ret; } -static eDriveModel GetDriveModel(int idx) +static enum eDriveModel GetDriveModel(int idx) { - eDriveModel eModel = UNKNOWN_MODEL; + enum eDriveModel eModel = UNKNOWN_MODEL; char path[512]; sprintf(path, fvendorid1, idx); @@ -121,24 +131,34 @@ static eDriveModel GetDriveModel(int idx) if (vendor_id == MICRON_VENDOR_ID) { switch (device_id) { case 0x5196: + fallthrough; case 0x51A0: + fallthrough; case 0x51A1: + fallthrough; case 0x51A2: eModel = M51AX; break; case 0x51B0: + fallthrough; case 0x51B1: + fallthrough; case 0x51B2: eModel = M51BX; break; case 0x51C0: + fallthrough; case 0x51C1: + fallthrough; case 0x51C2: + fallthrough; case 0x51C3: eModel = M51CX; break; case 0x5405: + fallthrough; case 0x5406: + fallthrough; case 0x5407: eModel = M5407; break; @@ -164,8 +184,7 @@ static int ZipAndRemoveDir(char *strDirName, char *strFileName) struct stat sb; if (strstr(strFileName, ".tar.gz") || strstr(strFileName, ".tgz")) { - sprintf(strBuffer, "tar -zcf \"%s\" \"%s\"", strFileName, - strDirName); + sprintf(strBuffer, "tar -zcf \"%s\" \"%s\"", strFileName, strDirName); is_tgz = true; } else { sprintf(strBuffer, "zip -r \"%s\" \"%s\" >temp.txt 2>&1", strFileName, @@ -211,47 +230,43 @@ static int SetupDebugDataDirectories(char *strSN, char *strFilePath, int k = 0; int i = 0; - if (strchr(strFilePath, '/') != NULL) { + if (strchr(strFilePath, '/')) { fileName = strrchr(strFilePath, '\\'); - if (fileName == NULL) { + if (!fileName) fileName = strrchr(strFilePath, '/'); - } - if (fileName != NULL) { - if (!strcmp(fileName, "/")) { + if (fileName) { + if (!strcmp(fileName, "/")) goto exit_status; - } while (strFilePath[nIndex] != '\0') { - if ('\\' == strFilePath[nIndex] && '\\' == strFilePath[nIndex + 1]) { + if ('\\' == strFilePath[nIndex] && '\\' == strFilePath[nIndex + 1]) goto exit_status; - } nIndex++; } length = (int)strlen(strFilePath) - (int)strlen(fileName); - if (fileName == strFilePath) { + if (fileName == strFilePath) length = 1; - } - if ((fileLocation = (char *)malloc(length + 1)) == NULL) { + fileLocation = (char *)malloc(length + 1); + if (!fileLocation) goto exit_status; - } strncpy(fileLocation, strFilePath, length); fileLocation[length] = '\0'; while (fileLocation[k] != '\0') { - if (fileLocation[k] == '\\') { + if (fileLocation[k] == '\\') fileLocation[k] = '/'; - } k++; } length = (int)strlen(fileLocation); if (':' == fileLocation[length - 1]) { - if ((strTemp = (char *)malloc(length + 2)) == NULL) { + strTemp = (char *)malloc(length + 2); + if (!strTemp) { free(fileLocation); goto exit_status; } @@ -260,7 +275,8 @@ static int SetupDebugDataDirectories(char *strSN, char *strFilePath, free(fileLocation); length = (int)strlen(strTemp); - if ((fileLocation = (char *)malloc(length + 1)) == NULL) { + fileLocation = (char *)malloc(length + 1); + if (!fileLocation) { free(strTemp); goto exit_status; } @@ -269,7 +285,7 @@ static int SetupDebugDataDirectories(char *strSN, char *strFilePath, free(strTemp); } - if (stat(fileLocation, &st) != 0) { + if (stat(fileLocation, &st)) { free(fileLocation); goto exit_status; } @@ -281,14 +297,13 @@ static int SetupDebugDataDirectories(char *strSN, char *strFilePath, nIndex = 0; for (i = 0; i < (int)strlen(strSN); i++) { - if (strSN[i] != ' ' && strSN[i] != '\n' && strSN[i] != '\t' && strSN[i] != '\r') { + if (strSN[i] != ' ' && strSN[i] != '\n' && strSN[i] != '\t' && strSN[i] != '\r') strMainDirName[nIndex++] = strSN[i]; - } } strMainDirName[nIndex] = '\0'; j = 1; - while (stat(strMainDirName, &dirStat) == 0) { + while (!stat(strMainDirName, &dirStat)) { strMainDirName[nIndex] = '\0'; sprintf(strAppend, "-%d", j); strcat(strMainDirName, strAppend); @@ -300,18 +315,18 @@ static int SetupDebugDataDirectories(char *strSN, char *strFilePath, goto exit_status; } - if (strOSDirName != NULL) { + if (strOSDirName) { sprintf(strOSDirName, "%s/%s", strMainDirName, "OS"); if (mkdir(strOSDirName, 0777) < 0) { rmdir(strMainDirName); err = -1; goto exit_status; + } } - } - if (strCtrlDirName != NULL) { + if (strCtrlDirName) { sprintf(strCtrlDirName, "%s/%s", strMainDirName, "Controller"); if (mkdir(strCtrlDirName, 0777) < 0) { - if (strOSDirName != NULL) + if (strOSDirName) rmdir(strOSDirName); rmdir(strMainDirName); err = -1; @@ -326,23 +341,22 @@ static int GetLogPageSize(int nFD, unsigned char ucLogID, int *nLogSize) { int err = 0; unsigned char pTmpBuf[CommonChunkSize] = { 0 }; - LogPageHeader_t *pLogHeader = NULL; + struct LogPageHeader_t *pLogHeader = NULL; if (ucLogID == 0xC1 || ucLogID == 0xC2 || ucLogID == 0xC4) { - err = nvme_get_log_simple(nFD, ucLogID, - CommonChunkSize, pTmpBuf); - if (err == 0) { - pLogHeader = (LogPageHeader_t *) pTmpBuf; - LogPageHeader_t *pLogHeader1 = (LogPageHeader_t *) pLogHeader; + err = nvme_get_log_simple(nFD, ucLogID, CommonChunkSize, pTmpBuf); + if (!err) { + pLogHeader = (struct LogPageHeader_t *) pTmpBuf; + struct LogPageHeader_t *pLogHeader1 = (struct LogPageHeader_t *) pLogHeader; *nLogSize = (int)(pLogHeader1->numDwordsInEntireLogPage) * 4; - if (pLogHeader1->logPageHeaderFormatVersion == 0) { - printf ("Unsupported log page format version %d of log page : 0x%X\n", - ucLogID, err); + if (!pLogHeader1->logPageHeaderFormatVersion) { + printf("Unsupported log page format version %d of log page : 0x%X\n", + ucLogID, err); *nLogSize = 0; err = -1; } } else { - printf ("Getting size of log page : 0x%X failed with %d (ignored)!\n", + printf("Getting size of log page : 0x%X failed with %d (ignored)!\n", ucLogID, err); *nLogSize = 0; } @@ -362,36 +376,32 @@ static int NVMEGetLogPage(int nFD, unsigned char ucLogID, unsigned char *pBuffer unsigned char *pTempPtr = pBuffer; unsigned char ucOpCode = 0x02; - if (ullBytesRead == 0 && (ucLogID == 0xE6 || ucLogID == 0xE7)) { + if (!ullBytesRead && (ucLogID == 0xE6 || ucLogID == 0xE7)) uiMaxChunk = 4096; - } else if (uiMaxChunk > 16 * 1024) { + else if (uiMaxChunk > 16 * 1024) uiMaxChunk = 16 * 1024; - } uiNumChunks = uiNumDwords / uiMaxChunk; - if (uiNumDwords % uiMaxChunk > 0) { + if (uiNumDwords % uiMaxChunk > 0) uiNumChunks += 1; - } for (unsigned int i = 0; i < uiNumChunks; i++) { memset(&cmd, 0, sizeof(cmd)); uiXferDwords = uiMaxChunk; - if (i == uiNumChunks - 1 && uiNumDwords % uiMaxChunk > 0) { + if (i == uiNumChunks - 1 && uiNumDwords % uiMaxChunk > 0) uiXferDwords = uiNumDwords % uiMaxChunk; - } cmd.opcode = ucOpCode; cmd.cdw10 |= ucLogID; cmd.cdw10 |= ((uiXferDwords - 1) & 0x0000FFFF) << 16; - if (ucLogID == 0x7) { + if (ucLogID == 0x7) cmd.cdw10 |= 0x80; - } - if (ullBytesRead == 0 && (ucLogID == 0xE6 || ucLogID == 0xE7)) { + if (!ullBytesRead && (ucLogID == 0xE6 || ucLogID == 0xE7)) cmd.cdw11 = 1; - } if (ullBytesRead > 0 && !(ucLogID == 0xE6 || ucLogID == 0xE7)) { unsigned long long ullOffset = ullBytesRead; + cmd.cdw12 = ullOffset & 0xFFFFFFFF; cmd.cdw13 = (ullOffset >> 32) & 0xFFFFFFFF; } @@ -413,15 +423,16 @@ static int NVMEResetLog(int nFD, unsigned char ucLogID, int nBufferSize, unsigned int *pBuffer = NULL; int err = 0; - if ((pBuffer = (unsigned int *)calloc(1, nBufferSize)) == NULL) + pBuffer = (unsigned int *)calloc(1, nBufferSize); + if (!pBuffer) return err; - while (err == 0 && llMaxSize > 0) { + while (!err && llMaxSize > 0) { err = NVMEGetLogPage(nFD, ucLogID, (unsigned char *)pBuffer, nBufferSize); if (err) { free(pBuffer); return err; - } + } if (pBuffer[0] == 0xdeadbeef) break; @@ -438,10 +449,10 @@ static int GetCommonLogPage(int nFD, unsigned char ucLogID, { unsigned char *pTempPtr = NULL; int err = 0; + pTempPtr = (unsigned char *)malloc(nBuffSize); - if (!pTempPtr) { + if (!pTempPtr) goto exit_status; - } memset(pTempPtr, 0, nBuffSize); err = nvme_get_log_simple(nFD, ucLogID, nBuffSize, pTempPtr); *pBuffer = pTempPtr; @@ -456,9 +467,9 @@ exit_status: static int micron_parse_options(struct nvme_dev **dev, int argc, char **argv, const char *desc, struct argconfig_commandline_options *opts, - eDriveModel *modelp) + enum eDriveModel *modelp) { - int idx = 0; + int idx; int err = parse_and_open(dev, argc, argv, desc, opts); if (err) { @@ -467,7 +478,8 @@ static int micron_parse_options(struct nvme_dev **dev, int argc, char **argv, } if (modelp) { - sscanf(argv[optind], "/dev/nvme%d", &idx); + if (sscanf(argv[optind], "/dev/nvme%d", &idx) != 1) + idx = 0; *modelp = GetDriveModel(idx); } @@ -490,7 +502,7 @@ static int micron_selective_download(int argc, char **argv, const char *desc = "This performs a selective firmware download, which allows the user to " "select which firmware binary to update for 9200 devices. This requires " - "a power cycle once the update completes. The options available are: \n\n" + "a power cycle once the update completes. The options available are:\n\n" "OOB - This updates the OOB and main firmware\n" "EEP - This updates the eeprom and main firmware\n" "ALL - This updates the eeprom, OOB, and main firmware"; @@ -525,30 +537,29 @@ static int micron_selective_download(int argc, char **argv, if (strlen(cfg.select) != 3) { fprintf(stderr, "Invalid select flag\n"); dev_close(dev); - return EINVAL; + return -EINVAL; } - for (int i = 0; i < 3; i++) { + for (int i = 0; i < 3; i++) cfg.select[i] = toupper(cfg.select[i]); - } - if (strncmp(cfg.select, "OOB", 3) == 0) { + if (!strncmp(cfg.select, "OOB", 3)) { selectNo = 18; - } else if (strncmp(cfg.select, "EEP", 3) == 0) { + } else if (!strncmp(cfg.select, "EEP", 3)) { selectNo = 10; - } else if (strncmp(cfg.select, "ALL", 3) == 0) { + } else if (!strncmp(cfg.select, "ALL", 3)) { selectNo = 26; } else { fprintf(stderr, "Invalid select flag\n"); dev_close(dev); - return EINVAL; + return -EINVAL; } fw_fd = open(cfg.fw, O_RDONLY); if (fw_fd < 0) { fprintf(stderr, "no firmware file provided\n"); dev_close(dev); - return EINVAL; + return -EINVAL; } err = fstat(fw_fd, &sb); @@ -572,28 +583,29 @@ static int micron_selective_download(int argc, char **argv, } if (read(fw_fd, fw_buf, fw_size) != ((ssize_t) (fw_size))) { - err = errno; - goto out_free; + err = errno; + goto out_free; } while (fw_size > 0) { xfer = min(xfer, fw_size); - struct nvme_fw_download_args args = { - .args_size = sizeof(args), - .fd = dev_fd(dev), - .offset = offset, - .data_len = xfer, - .data = fw_buf, - .timeout = NVME_DEFAULT_IOCTL_TIMEOUT, - .result = NULL, - }; + struct nvme_fw_download_args args = { + .args_size = sizeof(args), + .fd = dev_fd(dev), + .offset = offset, + .data_len = xfer, + .data = fw_buf, + .timeout = NVME_DEFAULT_IOCTL_TIMEOUT, + .result = NULL, + }; + err = nvme_fw_download(&args); if (err < 0) { perror("fw-download"); goto out_free; - } else if (err != 0) { - nvme_show_status(err); + } else if (err) { + nvme_show_status(err); goto out_free; } fw_buf += xfer; @@ -606,7 +618,7 @@ static int micron_selective_download(int argc, char **argv, if (err == 0x10B || err == 0x20B) { err = 0; fprintf(stderr, - "Update successful! Power cycle for changes to take effect\n"); + "Update successful! Power cycle for changes to take effect\n"); } out_free: @@ -624,12 +636,11 @@ static int micron_smbus_option(int argc, char **argv, __u32 cdw11 = 0; const char *desc = "Enable/Disable/Get status of SMBUS option on controller"; const char *option = "enable or disable or status"; - const char *value = "1 - hottest component temperature, 0 - composite " - "temperature (default) for enable option, 0 (current), " - "1 (default), 2 (saved) for status options"; + const char *value = + "1 - hottest component temperature, 0 - composite temperature (default) for enable option, 0 (current), 1 (default), 2 (saved) for status options"; const char *save = "1 - persistent, 0 - non-persistent (default)"; int fid = MICRON_FEATURE_SMBUS_OPTION; - eDriveModel model = UNKNOWN_MODEL; + enum eDriveModel model = UNKNOWN_MODEL; struct nvme_dev *dev; int err = 0; @@ -657,7 +668,7 @@ static int micron_smbus_option(int argc, char **argv, return err; if (model != M5407 && model != M5411) { - printf ("This option is not supported for specified drive\n"); + printf("This option is not supported for specified drive\n"); dev_close(dev); return err; } @@ -666,47 +677,43 @@ static int micron_smbus_option(int argc, char **argv, cdw11 = opt.value << 1 | 1; err = nvme_set_features_simple(dev_fd(dev), fid, 1, cdw11, opt.save, &result); - if (err == 0) { + if (!err) printf("successfully enabled SMBus on drive\n"); - } else { + else printf("Failed to enabled SMBus on drive\n"); - } - } - else if (!strcmp(opt.option, "status")) { - struct nvme_get_features_args args = { - .args_size = sizeof(args), - .fd = dev_fd(dev), - .fid = fid, - .nsid = 1, - .sel = opt.value, - .cdw11 = 0, - .uuidx = 0, - .data_len = 0, - .data = NULL, - .timeout = NVME_DEFAULT_IOCTL_TIMEOUT, - .result = &result, + } else if (!strcmp(opt.option, "status")) { + struct nvme_get_features_args args = { + .args_size = sizeof(args), + .fd = dev_fd(dev), + .fid = fid, + .nsid = 1, + .sel = opt.value, + .cdw11 = 0, + .uuidx = 0, + .data_len = 0, + .data = NULL, + .timeout = NVME_DEFAULT_IOCTL_TIMEOUT, + .result = &result, }; + err = nvme_get_features(&args); - if (err == 0) { - printf("SMBus status on the drive: %s (returns %s temperature) \n", - (result & 1) ? "enabled" : "disabled", - (result & 2) ? "hottest component" : "composite"); - } else { + if (!err) + printf("SMBus status on the drive: %s (returns %s temperature)\n", + (result & 1) ? "enabled" : "disabled", + (result & 2) ? "hottest component" : "composite"); + else printf("Failed to retrieve SMBus status on the drive\n"); - } - } - else if (!strcmp(opt.option, "disable")) { + } else if (!strcmp(opt.option, "disable")) { cdw11 = opt.value << 1 | 0; err = nvme_set_features_simple(dev_fd(dev), fid, 1, cdw11, opt.save, - &result); - if (err == 0) { + &result); + if (!err) printf("Successfully disabled SMBus on drive\n"); - } else { + else printf("Failed to disable SMBus on drive\n"); - } } else { printf("Invalid option %s, valid values are enable, disable or status\n", - opt.option); + opt.option); dev_close(dev); return -1; } @@ -742,32 +749,34 @@ static int micron_temp_stats(int argc, char **argv, struct command *cmd, err = parse_and_open(&dev, argc, argv, desc, opts); if (err) { - printf("\nDevice not found \n");; + printf("\nDevice not found\n"); return -1; } - if (strcmp(cfg.fmt, "json") == 0) + if (!strcmp(cfg.fmt, "json")) is_json = true; err = nvme_get_log_smart(dev_fd(dev), 0xffffffff, false, &smart_log); if (!err) { temperature = ((smart_log.temperature[1] << 8) | smart_log.temperature[0]); temperature = temperature ? temperature - 273 : 0; - for (i = 0; i < SensorCount && tempSensors[i] != 0; i++) { + for (i = 0; i < SensorCount && tempSensors[i]; i++) { tempSensors[i] = le16_to_cpu(smart_log.temp_sensor[i]); tempSensors[i] = tempSensors[i] ? tempSensors[i] - 273 : 0; } if (is_json) { struct json_object *stats = json_create_object(); char tempstr[64] = { 0 }; + root = json_create_object(); logPages = json_create_array(); json_object_add_value_array(root, "Micron temperature information", logPages); sprintf(tempstr, "%u C", temperature); json_object_add_value_string(stats, "Current Composite Temperature", tempstr); - for (i = 0; i < SensorCount && tempSensors[i] != 0; i++) { + for (i = 0; i < SensorCount && tempSensors[i]; i++) { char sensor_str[256] = { 0 }; char datastr[64] = { 0 }; + sprintf(sensor_str, "Temperature Sensor #%d", (i + 1)); sprintf(datastr, "%u C", tempSensors[i]); json_object_add_value_string(stats, sensor_str, datastr); @@ -779,9 +788,8 @@ static int micron_temp_stats(int argc, char **argv, struct command *cmd, } else { printf("Micron temperature information:\n"); printf("%-10s : %u C\n", "Current Composite Temperature", temperature); - for (i = 0; i < SensorCount && tempSensors[i] != 0; i++) { + for (i = 0; i < SensorCount && tempSensors[i]; i++) printf("%-10s%d : %u C\n", "Temperature Sensor #", i + 1, tempSensors[i]); - } } } dev_close(dev); @@ -791,7 +799,7 @@ static int micron_temp_stats(int argc, char **argv, struct command *cmd, static int micron_pcie_stats(int argc, char **argv, struct command *cmd, struct plugin *plugin) { - int i, err = 0, bus = 0, domain = 0, device = 0, function = 0, ctrlIdx; + int i, err = 0, bus, domain, device, function, ctrlIdx; char strTempFile[1024], strTempFile2[1024], command[1024]; struct nvme_dev *dev; char *businfo = NULL; @@ -802,7 +810,7 @@ static int micron_pcie_stats(int argc, char **argv, char correctable[8] = { 0 }; char uncorrectable[8] = { 0 }; struct nvme_passthru_cmd admin_cmd = { 0 }; - eDriveModel eModel = UNKNOWN_MODEL; + enum eDriveModel eModel = UNKNOWN_MODEL; char *res; bool is_json = true; bool counters = false; @@ -884,18 +892,20 @@ static int micron_pcie_stats(int argc, char **argv, err = parse_and_open(&dev, argc, argv, desc, opts); if (err) { - printf("\nDevice not found \n");; + printf("\nDevice not found\n"); return -1; } /* pull log details based on the model name */ - sscanf(argv[optind], "/dev/nvme%d", &ctrlIdx); - if ((eModel = GetDriveModel(ctrlIdx)) == UNKNOWN_MODEL) { - printf ("Unsupported drive model for vs-pcie-stats command\n"); + if (sscanf(argv[optind], "/dev/nvme%d", &ctrlIdx) != 1) + ctrlIdx = 0; + eModel = GetDriveModel(ctrlIdx); + if (eModel == UNKNOWN_MODEL) { + printf("Unsupported drive model for vs-pcie-stats command\n"); goto out; } - if (strcmp(cfg.fmt, "normal") == 0) + if (!strcmp(cfg.fmt, "normal")) is_json = false; if (eModel == M5407) { @@ -941,16 +951,17 @@ static int micron_pcie_stats(int argc, char **argv, } } businfo = strrchr(strTempFile2, '/'); - sscanf(businfo, "/%x:%x:%x.%x", &domain, &bus, &device, &function); + if (sscanf(businfo, "/%x:%x:%x.%x", &domain, &bus, &device, &function) != 4) + domain = bus = device = function = 0; sprintf(command, "setpci -s %x:%x.%x ECAP_AER+10.L", bus, device, function); fp = popen(command, "r"); - if (fp == NULL) { + if (!fp) { printf("Failed to retrieve error count\n"); goto out; } res = fgets(correctable, sizeof(correctable), fp); - if (res == NULL) { + if (!res) { printf("Failed to retrieve error count\n"); pclose(fp); goto out; @@ -960,12 +971,12 @@ static int micron_pcie_stats(int argc, char **argv, sprintf(command, "setpci -s %x:%x.%x ECAP_AER+0x4.L", bus, device, function); fp = popen(command, "r"); - if (fp == NULL) { + if (!fp) { printf("Failed to retrieve error count\n"); goto out; } res = fgets(uncorrectable, sizeof(uncorrectable), fp); - if (res == NULL) { + if (!res) { printf("Failed to retrieve error count\n"); pclose(fp); goto out; @@ -977,19 +988,18 @@ static int micron_pcie_stats(int argc, char **argv, print_stats: if (is_json) { - struct json_object *root = json_create_object(); struct json_object *pcieErrors = json_create_array(); struct json_object *stats = json_create_object(); __u8 *pcounter = (__u8 *)&pcie_error_counters; json_object_add_value_array(root, "PCIE Stats", pcieErrors); - for (i = 0; i < sizeof(pcie_correctable_errors) / sizeof(pcie_correctable_errors[0]); i++) { + for (i = 0; i < ARRAY_SIZE(pcie_correctable_errors); i++) { __u16 val = counters ? *(__u16 *)(pcounter + pcie_correctable_errors[i].val) : (correctable_errors >> pcie_correctable_errors[i].bit) & 1; json_object_add_value_int(stats, pcie_correctable_errors[i].err, val); } - for (i = 0; i < sizeof(pcie_uncorrectable_errors) / sizeof(pcie_uncorrectable_errors[0]); i++) { + for (i = 0; i < ARRAY_SIZE(pcie_uncorrectable_errors); i++) { __u16 val = counters ? *(__u16 *)(pcounter + pcie_uncorrectable_errors[i].val) : (uncorrectable_errors >> pcie_uncorrectable_errors[i].bit) & 1; json_object_add_value_int(stats, pcie_uncorrectable_errors[i].err, val); @@ -1000,23 +1010,20 @@ print_stats: json_free_object(root); } else if (counters == true) { __u8 *pcounter = (__u8 *)&pcie_error_counters; - for (i = 0; i < sizeof(pcie_correctable_errors) / sizeof(pcie_correctable_errors[0]); i++) { + + for (i = 0; i < ARRAY_SIZE(pcie_correctable_errors); i++) printf("%-42s : %-1hu\n", pcie_correctable_errors[i].err, *(__u16 *)(pcounter + pcie_correctable_errors[i].val)); - } - for (i = 0; i < sizeof(pcie_uncorrectable_errors) / sizeof(pcie_uncorrectable_errors[0]); i++) { + for (i = 0; i < ARRAY_SIZE(pcie_uncorrectable_errors); i++) printf("%-42s : %-1hu\n", pcie_uncorrectable_errors[i].err, *(__u16 *)(pcounter + pcie_uncorrectable_errors[i].val)); - } } else if (eModel == M5407 || eModel == M5410) { - for (i = 0; i < sizeof(pcie_correctable_errors) / sizeof(pcie_correctable_errors[0]); i++) { + for (i = 0; i < ARRAY_SIZE(pcie_correctable_errors); i++) printf("%-42s : %-1d\n", pcie_correctable_errors[i].err, ((correctable_errors >> pcie_correctable_errors[i].bit) & 1)); - } - for (i = 0; i < sizeof(pcie_uncorrectable_errors) / sizeof(pcie_uncorrectable_errors[0]); i++) { + for (i = 0; i < ARRAY_SIZE(pcie_uncorrectable_errors); i++) printf("%-42s : %-1d\n", pcie_uncorrectable_errors[i].err, ((uncorrectable_errors >> pcie_uncorrectable_errors[i].bit) & 1)); - } } else { printf("PCIE Stats:\n"); printf("Device correctable errors detected: %s\n", correctable); @@ -1032,14 +1039,14 @@ static int micron_clear_pcie_correctable_errors(int argc, char **argv, struct command *cmd, struct plugin *plugin) { - int err = -EINVAL, bus = 0, domain = 0, device = 0, function = 0; + int err = -EINVAL, bus, domain, device, function; char strTempFile[1024], strTempFile2[1024], command[1024]; struct nvme_dev *dev; char *businfo = NULL; char *devicename = NULL; char tdevice[PATH_MAX] = { 0 }; ssize_t sLinkSize = 0; - eDriveModel model = UNKNOWN_MODEL; + enum eDriveModel model = UNKNOWN_MODEL; struct nvme_passthru_cmd admin_cmd = { 0 }; char correctable[8] = { 0 }; FILE *fp; @@ -1047,6 +1054,7 @@ static int micron_clear_pcie_correctable_errors(int argc, char **argv, const char *desc = "Clear PCIe Device Correctable Errors"; __u32 result = 0; __u8 fid = MICRON_FEATURE_CLEAR_PCI_CORRECTABLE_ERRORS; + OPT_ARGS(opts) = { OPT_END() }; @@ -1057,9 +1065,11 @@ static int micron_clear_pcie_correctable_errors(int argc, char **argv, /* For M51CX models, PCIe errors are cleared using 0xC3 feature */ if (model == M51CX) { - err = nvme_set_features_simple(dev_fd(dev), fid, 0, (1 << 31), false, - &result); - if (err == 0 && (err = (int)result) == 0) { + err = nvme_set_features_simple(dev_fd(dev), fid, 0, (1 << 31), false, + &result); + if (!err) + err = (int)result; + if (!err) { printf("Device correctable errors are cleared!\n"); goto out; } @@ -1068,12 +1078,11 @@ static int micron_clear_pcie_correctable_errors(int argc, char **argv, admin_cmd.addr = 0; admin_cmd.cdw10 = 0; err = nvme_submit_admin_passthru(dev_fd(dev), &admin_cmd, NULL); - if (err == 0) { + if (!err) { printf("Device correctable error counters are cleared!\n"); goto out; } else { - /* proceed to clear status bits using sysfs interface - printf("Error clearing PCIe correctable errors = 0x%x\n", err); */ + /* proceed to clear status bits using sysfs interface */ } } @@ -1113,12 +1122,13 @@ static int micron_clear_pcie_correctable_errors(int argc, char **argv, } } businfo = strrchr(strTempFile2, '/'); - sscanf(businfo, "/%x:%x:%x.%x", &domain, &bus, &device, &function); + if (sscanf(businfo, "/%x:%x:%x.%x", &domain, &bus, &device, &function) != 4) + domain = bus = device = function = 0; sprintf(command, "setpci -s %x:%x.%x ECAP_AER+0x10.L=0xffffffff", bus, device, function); err = -1; fp = popen(command, "r"); - if (fp == NULL) { + if (!fp) { printf("Failed to clear error count\n"); goto out; } @@ -1127,12 +1137,12 @@ static int micron_clear_pcie_correctable_errors(int argc, char **argv, sprintf(command, "setpci -s %x:%x.%x ECAP_AER+0x10.L", bus, device, function); fp = popen(command, "r"); - if (fp == NULL) { + if (!fp) { printf("Failed to retrieve error count\n"); goto out; } res = fgets(correctable, sizeof(correctable), fp); - if (res == NULL) { + if (!res) { printf("Failed to retrieve error count\n"); pclose(fp); goto out; @@ -1172,9 +1182,9 @@ static void init_d0_log_page(__u8 *buf, __u8 nsze) count_hi = ((__u64)logD0[39] << 32) | logD0[38]; count_lo = ((__u64)logD0[37] << 32) | logD0[36]; - if (count_hi != 0) + if (count_hi) sprintf(d0_log_page[1].datastr, "0x%"PRIx64"%016"PRIx64, - le64_to_cpu(count_hi), le64_to_cpu(count_lo)); + le64_to_cpu(count_hi), le64_to_cpu(count_lo)); else sprintf(d0_log_page[1].datastr, "0x%"PRIx64, le64_to_cpu(count_lo)); @@ -1318,14 +1328,16 @@ fb_log_page[] = { { "Log Page GUID", 0, 16}, }; -/* Common function to print Micron VS log pages */ -static void print_micron_vs_logs( - __u8 *buf, /* raw log data */ - struct micron_vs_logpage *log_page, /* format of the data */ - int field_count, /* log field count */ - struct json_object *stats, /* json object to add fields */ - __u8 spec /* ocp spec index */ -) +/* + * Common function to print Micron VS log pages + * - buf: raw log data + * - log_page: format of the data + * - field_count: log field count + * - stats: json object to add fields + * - spec: ocp spec index + */ +static void print_micron_vs_logs(__u8 *buf, struct micron_vs_logpage *log_page, int field_count, + struct json_object *stats, __u8 spec) { __u64 lval_lo, lval_hi; __u32 ival; @@ -1337,8 +1349,10 @@ static void print_micron_vs_logs( for (field = 0; field < field_count; field++) { char datastr[1024] = { 0 }; char *sfield = NULL; - int size = (spec == 0) ? log_page[field].size : log_page[field].size2; - if (size == 0) continue; + int size = !spec ? log_page[field].size : log_page[field].size2; + + if (!size) + continue; sfield = log_page[field].field; if (size == 16) { if (strstr(sfield, "GUID")) { @@ -1383,11 +1397,10 @@ static void print_micron_vs_logs( /* do not print reserved values */ if (strstr(sfield, "Reserved")) continue; - if (stats != NULL) { + if (stats) json_object_add_value_string(stats, sfield, datastr); - } else { + else printf("%-40s : %-4s\n", sfield, datastr); - } } } @@ -1396,7 +1409,7 @@ static void print_smart_cloud_health_log(__u8 *buf, bool is_json) struct json_object *root; struct json_object *logPages; struct json_object *stats = NULL; - int field_count = sizeof(ocp_c0_log_page)/sizeof(ocp_c0_log_page[0]); + int field_count = ARRAY_SIZE(ocp_c0_log_page); if (is_json) { root = json_create_object(); @@ -1421,7 +1434,7 @@ static void print_nand_stats_fb(__u8 *buf, __u8 *buf2, __u8 nsze, bool is_json, struct json_object *root; struct json_object *logPages; struct json_object *stats = NULL; - int field_count = sizeof(fb_log_page)/sizeof(fb_log_page[0]); + int field_count = ARRAY_SIZE(fb_log_page); if (is_json) { root = json_create_object(); @@ -1434,19 +1447,17 @@ static void print_nand_stats_fb(__u8 *buf, __u8 *buf2, __u8 nsze, bool is_json, print_micron_vs_logs(buf, fb_log_page, field_count, stats, spec); /* print last three entries from D0 log page */ - if (buf2 != NULL) { + if (buf2) { init_d0_log_page(buf2, nsze); if (is_json) { - for (int i = 0; i < 7; i++) { + for (int i = 0; i < 7; i++) json_object_add_value_string(stats, d0_log_page[i].field, d0_log_page[i].datastr); - } } else { - for (int i = 0; i < 7; i++) { + for (int i = 0; i < 7; i++) printf("%-40s : %s\n", d0_log_page[i].field, d0_log_page[i].datastr); - } } } @@ -1471,24 +1482,22 @@ static void print_nand_stats_d0(__u8 *buf, __u8 oacs, bool is_json) "Extended Smart Log Page : 0xD0", logPages); - for (int i = 0; i < 7; i++) { + for (int i = 0; i < 7; i++) json_object_add_value_string(stats, d0_log_page[i].field, d0_log_page[i].datastr); - } json_array_add_value_object(logPages, stats); json_print_object(root, NULL); printf("\n"); json_free_object(root); } else { - for (int i = 0; i < 7; i++) { + for (int i = 0; i < 7; i++) printf("%-40s : %s\n", d0_log_page[i].field, d0_log_page[i].datastr); - } } } -static bool nsze_from_oacs = false; /* read nsze for now from idd[4059] */ +static bool nsze_from_oacs; /* read nsze for now from idd[4059] */ static int micron_nand_stats(int argc, char **argv, struct command *cmd, struct plugin *plugin) @@ -1496,7 +1505,7 @@ static int micron_nand_stats(int argc, char **argv, const char *desc = "Retrieve Micron NAND stats for the given device "; unsigned int extSmartLog[D0_log_size/sizeof(int)] = { 0 }; unsigned int logFB[FB_log_size/sizeof(int)] = { 0 }; - eDriveModel eModel = UNKNOWN_MODEL; + enum eDriveModel eModel = UNKNOWN_MODEL; struct nvme_id_ctrl ctrl; struct nvme_dev *dev; int err, ctrlIdx; @@ -1519,11 +1528,11 @@ static int micron_nand_stats(int argc, char **argv, err = parse_and_open(&dev, argc, argv, desc, opts); if (err) { - printf("\nDevice not found \n");; + printf("\nDevice not found\n"); return -1; } - if (strcmp(cfg.fmt, "normal") == 0) + if (!strcmp(cfg.fmt, "normal")) is_json = false; err = nvme_identify_ctrl(dev_fd(dev), &ctrl); @@ -1533,29 +1542,31 @@ static int micron_nand_stats(int argc, char **argv, } /* pull log details based on the model name */ - sscanf(argv[optind], "/dev/nvme%d", &ctrlIdx); + if (sscanf(argv[optind], "/dev/nvme%d", &ctrlIdx) != 1) + ctrlIdx = 0; eModel = GetDriveModel(ctrlIdx); if ((eModel == UNKNOWN_MODEL) || (eModel == M51CX)) { - printf ("Unsupported drive model for vs-nand-stats command\n"); - err = -1; + printf("Unsupported drive model for vs-nand-stats command\n"); + err = -1; goto out; } err = nvme_get_log_simple(dev_fd(dev), 0xD0, D0_log_size, extSmartLog); - has_d0_log = (0 == err); + has_d0_log = !err; /* should check for firmware version if this log is supported or not */ if (eModel == M5407 || eModel == M5410) { err = nvme_get_log_simple(dev_fd(dev), 0xFB, FB_log_size, logFB); - has_fb_log = (0 == err); + has_fb_log = !err; } nsze = (ctrl.vs[987] == 0x12); - if (nsze == 0 && nsze_from_oacs) + if (!nsze && nsze_from_oacs) nsze = ((ctrl.oacs >> 3) & 0x1); err = 0; if (has_fb_log) { __u8 spec = (eModel == M5410) ? 0 : 1; /* FB spec version */ + print_nand_stats_fb((__u8 *)logFB, (__u8 *)extSmartLog, nsze, is_json, spec); } else if (has_d0_log) { print_nand_stats_d0((__u8 *)extSmartLog, nsze, is_json); @@ -1566,7 +1577,7 @@ static int micron_nand_stats(int argc, char **argv, out: dev_close(dev); if (err > 0) - nvme_show_status(err); + nvme_show_status(err); return err; } @@ -1576,15 +1587,14 @@ static void print_ext_smart_logs_e1(__u8 *buf, bool is_json) struct json_object *root; struct json_object *logPages; struct json_object *stats = NULL; - int field_count = sizeof(e1_log_page)/sizeof(e1_log_page[0]); + int field_count = ARRAY_SIZE(e1_log_page); if (is_json) { root = json_create_object(); stats = json_create_object(); logPages = json_create_array(); json_object_add_value_array(root, "SMART Extended Log:0xE1", logPages); - } - else { + } else { printf("SMART Extended Log:0xE1\n"); } @@ -1603,8 +1613,8 @@ static int micron_smart_ext_log(int argc, char **argv, { const char *desc = "Retrieve extended SMART logs for the given device "; unsigned int extSmartLog[E1_log_size/sizeof(int)] = { 0 }; - eDriveModel eModel = UNKNOWN_MODEL; - int err = 0, ctrlIdx = 0; + enum eDriveModel eModel = UNKNOWN_MODEL; + int err = 0, ctrlIdx; struct nvme_dev *dev; bool is_json = true; struct format { @@ -1621,22 +1631,23 @@ static int micron_smart_ext_log(int argc, char **argv, err = parse_and_open(&dev, argc, argv, desc, opts); if (err) { - printf("\nDevice not found \n");; + printf("\nDevice not found\n"); return -1; } - if (strcmp(cfg.fmt, "normal") == 0) + if (!strcmp(cfg.fmt, "normal")) is_json = false; - sscanf(argv[optind], "/dev/nvme%d", &ctrlIdx); - if ((eModel = GetDriveModel(ctrlIdx)) != M51CX) { - printf ("Unsupported drive model for vs-smart-ext-log command\n"); + if (sscanf(argv[optind], "/dev/nvme%d", &ctrlIdx) != 1) + ctrlIdx = 0; + eModel = GetDriveModel(ctrlIdx); + if (eModel != M51CX) { + printf("Unsupported drive model for vs-smart-ext-log command\n"); err = -1; goto out; } err = nvme_get_log_simple(dev_fd(dev), 0xE1, E1_log_size, extSmartLog); - if (!err) { + if (!err) print_ext_smart_logs_e1((__u8 *)extSmartLog, is_json); - } out: dev_close(dev); @@ -1705,7 +1716,7 @@ static void GetTimestampInfo(const char *strOSDirName) t = time(NULL); tmp = localtime(&t); - if (tmp == NULL) + if (!tmp) return; num = strftime((char *)outstr, sizeof(outstr), @@ -1721,17 +1732,17 @@ static void GetTimestampInfo(const char *strOSDirName) static void GetCtrlIDDInfo(const char *dir, struct nvme_id_ctrl *ctrlp) { - WriteData((__u8*)ctrlp, sizeof(*ctrlp), dir, + WriteData((__u8 *)ctrlp, sizeof(*ctrlp), dir, "nvme_controller_identify_data.bin", "id-ctrl"); } static void GetSmartlogData(int fd, const char *dir) { struct nvme_smart_log smart_log; - if (nvme_get_log_smart(fd, -1, false, &smart_log) == 0) { - WriteData((__u8*)&smart_log, sizeof(smart_log), dir, - "smart_data.bin", "smart log"); - } + + if (!nvme_get_log_smart(fd, -1, false, &smart_log)) + WriteData((__u8 *)&smart_log, sizeof(smart_log), dir, + "smart_data.bin", "smart log"); } static void GetErrorlogData(int fd, int entries, const char *dir) @@ -1740,13 +1751,12 @@ static void GetErrorlogData(int fd, int entries, const char *dir) struct nvme_error_log_page *error_log = (struct nvme_error_log_page *)calloc(1, logSize); - if (error_log == NULL) + if (!error_log) return; - if (nvme_get_log_error(fd, entries, false, error_log) == 0) { - WriteData((__u8*)error_log, logSize, dir, - "error_information_log.bin", "error log"); - } + if (!nvme_get_log_error(fd, entries, false, error_log)) + WriteData((__u8 *)error_log, logSize, dir, + "error_information_log.bin", "error log"); free(error_log); } @@ -1759,26 +1769,23 @@ static void GetGenericLogs(int fd, const char *dir) struct nvme_persistent_event_log pevent_log; void *pevent_log_info = NULL; __u32 log_len = 0; - int err = 0 ; + int err = 0; bool huge = false; /* get self test log */ - if (nvme_get_log_device_self_test(fd, &self_test_log) == 0) { - WriteData((__u8*)&self_test_log, sizeof(self_test_log), dir, - "drive_self_test.bin", "self test log"); - } + if (!nvme_get_log_device_self_test(fd, &self_test_log)) + WriteData((__u8 *)&self_test_log, sizeof(self_test_log), dir, + "drive_self_test.bin", "self test log"); /* get fw slot info log */ - if (nvme_get_log_fw_slot(fd, false, &fw_log) == 0) { - WriteData((__u8*)&fw_log, sizeof(fw_log), dir, - "firmware_slot_info_log.bin", "firmware log"); - } + if (!nvme_get_log_fw_slot(fd, false, &fw_log)) + WriteData((__u8 *)&fw_log, sizeof(fw_log), dir, + "firmware_slot_info_log.bin", "firmware log"); /* get effects log */ - if (nvme_get_log_cmd_effects(fd, NVME_CSI_NVM, &effects) == 0) { - WriteData((__u8*)&effects, sizeof(effects), dir, - "command_effects_log.bin", "effects log"); - } + if (!nvme_get_log_cmd_effects(fd, NVME_CSI_NVM, &effects)) + WriteData((__u8 *)&effects, sizeof(effects), dir, + "command_effects_log.bin", "effects log"); /* get persistent event log */ (void)nvme_get_log_persistent_event(fd, NVME_PEVENT_LOG_RELEASE_CTX, @@ -1799,12 +1806,10 @@ static void GetGenericLogs(int fd, const char *dir) } err = nvme_get_log_persistent_event(fd, NVME_PEVENT_LOG_READ, log_len, pevent_log_info); - if (err == 0) { - WriteData((__u8*)pevent_log_info, log_len, dir, - "persistent_event_log.bin", "persistent event log"); - } + if (!err) + WriteData((__u8 *)pevent_log_info, log_len, dir, + "persistent_event_log.bin", "persistent event log"); nvme_free(pevent_log_info, huge); - return; } static void GetNSIDDInfo(int fd, const char *dir, int nsid) @@ -1812,9 +1817,9 @@ static void GetNSIDDInfo(int fd, const char *dir, int nsid) char file[PATH_MAX] = { 0 }; struct nvme_id_ns ns; - if (nvme_identify_ns(fd, nsid, &ns) == 0) { + if (!nvme_identify_ns(fd, nsid, &ns)) { sprintf(file, "identify_namespace_%d_data.bin", nsid); - WriteData((__u8*)&ns, sizeof(ns), dir, file, "id-ns"); + WriteData((__u8 *)&ns, sizeof(ns), dir, file, "id-ns"); } } @@ -1844,7 +1849,7 @@ static void GetOSConfig(const char *strOSDirName) for (i = 0; i < 7; i++) { fpOSConfig = fopen(strFileName, "a+"); - if (NULL != fpOSConfig) { + if (fpOSConfig) { fprintf(fpOSConfig, "\n\n\n\n%s\n-----------------------------------------------\n", cmdArray[i].strcmdHeader); @@ -1866,17 +1871,17 @@ static int micron_telemetry_log(int fd, __u8 type, __u8 **data, unsigned char ctrl_init = (type == 0x8); __u8 *buffer = (unsigned char *)calloc(bs, 1); - if (buffer == NULL) + + if (!buffer) return -1; if (ctrl_init) - err = nvme_get_log_telemetry_ctrl(fd, true, 0, bs, buffer); + err = nvme_get_log_telemetry_ctrl(fd, true, 0, bs, buffer); else - err = nvme_get_log_telemetry_host(fd, 0, bs, buffer); - if (err != 0) { + err = nvme_get_log_telemetry_host(fd, 0, bs, buffer); + if (err) { fprintf(stderr, "Failed to get telemetry log header for 0x%X\n", type); - if (buffer != NULL) { + if (buffer) free(buffer); - } return err; } @@ -1887,9 +1892,9 @@ static int micron_telemetry_log(int fd, __u8 type, __u8 **data, data_area[0] = data_area[1] > data_area[2] ? data_area[1] : data_area[2]; data_area[0] = data_area[3] > data_area[0] ? data_area[3] : data_area[0]; - if (data_area[da] == 0) { + if (!data_area[da]) { fprintf(stderr, "Requested telemetry data for 0x%X is empty\n", type); - if (buffer != NULL) { + if (buffer) { free(buffer); buffer = NULL; } @@ -1899,21 +1904,22 @@ static int micron_telemetry_log(int fd, __u8 type, __u8 **data, *logSize = data_area[da] * bs; offset = bs; err = 0; - if ((buffer = (unsigned char *)realloc(buffer, (size_t)(*logSize))) != NULL) { - while (err == 0 && offset != *logSize) { - if (ctrl_init) - err = nvme_get_log_telemetry_ctrl(fd, true, 0, *logSize, buffer + offset); - else - err = nvme_get_log_telemetry_host(fd, 0, *logSize, buffer + offset); - offset += bs; + buffer = (unsigned char *)realloc(buffer, (size_t)(*logSize)); + if (buffer) { + while (!err && offset != *logSize) { + if (ctrl_init) + err = nvme_get_log_telemetry_ctrl(fd, true, 0, *logSize, buffer + offset); + else + err = nvme_get_log_telemetry_host(fd, 0, *logSize, buffer + offset); + offset += bs; } } - if (err == 0 && buffer != NULL) { + if (!err && buffer) { *data = buffer; } else { fprintf(stderr, "Failed to get telemetry data for 0x%x\n", type); - if (buffer != NULL) + if (buffer) free(buffer); } @@ -1924,7 +1930,7 @@ static int GetTelemetryData(int fd, const char *dir) { unsigned char *buffer = NULL; int i, err, logSize = 0; - char msg[256] = {0}; + char msg[256] = { 0 }; struct { __u8 log; char *file; @@ -1933,9 +1939,9 @@ static int GetTelemetryData(int fd, const char *dir) {0x08, "nvmetelemetrylog.bin"}, }; - for(i = 0; i < (int)(sizeof(tmap)/sizeof(tmap[0])); i++) { + for (i = 0; i < (int)(ARRAY_SIZE(tmap)); i++) { err = micron_telemetry_log(fd, tmap[i].log, &buffer, &logSize, 0); - if (err == 0 && logSize > 0 && buffer != NULL) { + if (!err && logSize > 0 && buffer) { sprintf(msg, "telemetry log: 0x%X", tmap[i].log); WriteData(buffer, logSize, dir, tmap[i].file, msg); } @@ -1973,7 +1979,7 @@ static int GetFeatureSettings(int fd, const char *dir) {0x80, "nvme_feature_setting_sw_progress_marker.bin"}, }; - for (i = 0; i < (int)(sizeof(fmap)/sizeof(fmap[0])); i++) { + for (i = 0; i < (int)(ARRAY_SIZE(fmap)); i++) { if (fmap[i].id == 0x03) { len = 4096; bufp = (unsigned char *)(&buf[0]); @@ -1996,19 +2002,18 @@ static int GetFeatureSettings(int fd, const char *dir) .result = &attrVal, }; err = nvme_get_features(&args); - if (err == 0) { + if (!err) { sprintf(msg, "feature: 0x%X", fmap[i].id); - WriteData((__u8*)&attrVal, sizeof(attrVal), dir, fmap[i].file, msg); - if (bufp != NULL) { + WriteData((__u8 *)&attrVal, sizeof(attrVal), dir, fmap[i].file, msg); + if (bufp) WriteData(bufp, len, dir, fmap[i].file, msg); - } } else { fprintf(stderr, "Feature 0x%x data not retrieved, error %d (ignored)!\n", fmap[i].id, err); errcnt++; } } - return (int)(errcnt == sizeof(fmap)/sizeof(fmap[0])); + return (int)(errcnt == ARRAY_SIZE(fmap)); } static int micron_drive_info(int argc, char **argv, struct command *cmd, @@ -2024,7 +2029,7 @@ static int micron_drive_info(int argc, char **argv, struct command *cmd, unsigned char bs_ver_major; unsigned char bs_ver_minor; } dinfo = { 0 }; - eDriveModel model = UNKNOWN_MODEL; + enum eDriveModel model = UNKNOWN_MODEL; bool is_json = false; struct json_object *root, *driveInfo; struct nvme_dev *dev; @@ -2053,7 +2058,7 @@ static int micron_drive_info(int argc, char **argv, struct command *cmd, return -1; } - if (strcmp(cfg.fmt, "json") == 0) + if (!strcmp(cfg.fmt, "json")) is_json = true; if (model == M5407) { @@ -2082,19 +2087,20 @@ static int micron_drive_info(int argc, char **argv, struct command *cmd, if (is_json) { struct json_object *pinfo = json_create_object(); char tempstr[64] = { 0 }; + root = json_create_object(); driveInfo = json_create_array(); json_object_add_value_array(root, "Micron Drive HW Information", driveInfo); - sprintf(tempstr, "%hhu.%hhu", dinfo.hw_ver_major, dinfo.hw_ver_minor); + sprintf(tempstr, "%u.%u", dinfo.hw_ver_major, dinfo.hw_ver_minor); json_object_add_value_string(pinfo, "Drive Hardware Version", tempstr); if (dinfo.ftl_unit_size) { - sprintf(tempstr, "%hhu KB", dinfo.ftl_unit_size); + sprintf(tempstr, "%u KB", dinfo.ftl_unit_size); json_object_add_value_string(pinfo, "FTL_unit_size", tempstr); } - if (dinfo.bs_ver_major != 0 || dinfo.bs_ver_minor != 0) { - sprintf(tempstr, "%hhu.%hhu", dinfo.bs_ver_major, dinfo.bs_ver_minor); + if (dinfo.bs_ver_major || dinfo.bs_ver_minor) { + sprintf(tempstr, "%u.%u", dinfo.bs_ver_major, dinfo.bs_ver_minor); json_object_add_value_string(pinfo, "Boot Spec.Version", tempstr); } @@ -2103,16 +2109,15 @@ static int micron_drive_info(int argc, char **argv, struct command *cmd, printf("\n"); json_free_object(root); } else { - printf("Drive Hardware Version: %hhu.%hhu\n", + printf("Drive Hardware Version: %u.%u\n", dinfo.hw_ver_major, dinfo.hw_ver_minor); if (dinfo.ftl_unit_size) - printf("FTL_unit_size: %hhu KB\n", dinfo.ftl_unit_size); + printf("FTL_unit_size: %u KB\n", dinfo.ftl_unit_size); - if (dinfo.bs_ver_major != 0 || dinfo.bs_ver_minor != 0) { - printf("Boot Spec.Version: %hhu.%hhu\n", - dinfo.bs_ver_major, dinfo.bs_ver_minor); - } + if (dinfo.bs_ver_major || dinfo.bs_ver_minor) + printf("Boot Spec.Version: %u.%u\n", + dinfo.bs_ver_major, dinfo.bs_ver_minor); } dev_close(dev); @@ -2136,7 +2141,7 @@ static int micron_plugin_version(int argc, char **argv, struct command *cmd, } /* Binary format of firmware activation history entry */ -struct __attribute__((__packed__)) fw_activation_history_entry { +struct __packed fw_activation_history_entry { __u8 version; __u8 length; __u16 rsvd1; @@ -2153,7 +2158,7 @@ struct __attribute__((__packed__)) fw_activation_history_entry { }; /* Binary format for firmware activation history table */ -struct __attribute__((__packed__)) micron_fw_activation_history_table { +struct __packed micron_fw_activation_history_table { __u8 log_page; __u8 rsvd1[3]; __le32 num_entries; @@ -2163,35 +2168,18 @@ struct __attribute__((__packed__)) micron_fw_activation_history_table { __u8 GUID[16]; }; -/* header to be printed field widths = 10 | 12 | 10 | 11 | 12 | 9 | 9 | 9 */ - -const char *fw_activation_history_table_header = "\ -__________________________________________________________________________________\n\ - | | | | | | | \n\ -Firmware | Power On | Power | Previous | New FW | Slot | Commit | Result \n\ -Activation| Hour | cycle | firmware | activated | number | Action | \n\ -Counter | | count | | | | Type | \n\ -__________|___________|_________|__________|___________|________|________|________\n"; - -static int display_fw_activate_entry ( - int entry_count, - struct fw_activation_history_entry *entry, - char *formatted_entry, - struct json_object *stats -) +static int display_fw_activate_entry(int entry_count, struct fw_activation_history_entry *entry, + char *formatted_entry, struct json_object *stats) { - time_t timestamp, hours; - char buffer[32]; - __u8 minutes, seconds; - char *ca[] = {"000b", "001b", "010b", "011b"}; - char *ptr = formatted_entry; - int index = 0, entry_size = 82; - - if ((entry->version != 1 && entry->version != 2) || entry->length != 64) { - /*fprintf(stderr, "unsupported entry ! version: %x with length: %d\n", - entry->version, entry->length); */ + time_t timestamp, hours; + char buffer[32]; + __u8 minutes, seconds; + static const char * const ca[] = {"000b", "001b", "010b", "011b"}; + char *ptr = formatted_entry; + int index = 0, entry_size = 82; + + if ((entry->version != 1 && entry->version != 2) || entry->length != 64) return -EINVAL; - } sprintf(ptr, "%d", entry_count); ptr += 10; @@ -2200,7 +2188,7 @@ static int display_fw_activate_entry ( hours = timestamp / 3600; minutes = (timestamp % 3600) / 60; seconds = (timestamp % 3600) % 60; - sprintf(ptr, "|%"PRIu64":%hhu:%hhu", (uint64_t)hours, minutes, seconds); + sprintf(ptr, "|%"PRIu64":%u:%u", (uint64_t)hours, minutes, seconds); ptr += 12; sprintf(ptr, "| %"PRIu64, le64_to_cpu(entry->power_cycle_count)); @@ -2228,11 +2216,10 @@ static int display_fw_activate_entry ( ptr += 9; /* result */ - if (entry->result) { + if (entry->result) sprintf(ptr, "| Fail #%d", entry->result); - } else { + else sprintf(ptr, "| pass"); - } /* replace all null charecters with spaces */ ptr = formatted_entry; @@ -2244,6 +2231,16 @@ static int display_fw_activate_entry ( return 0; } +static void micron_fw_activation_history_header_print(void) +{ + /* header to be printed field widths = 10 | 12 | 10 | 11 | 12 | 9 | 9 | 9 */ + printf("__________________________________________________________________________________\n"); + printf(" | | | | | | |\n"); + printf("Firmware | Power On | Power | Previous | New FW | Slot | Commit | Result\n"); + printf("Activation| Hour | cycle | firmware | activated | number | Action |\n"); + printf("Counter | | count | | | | Type |\n"); + printf("__________|___________|_________|__________|___________|________|________|________\n"); +} static int micron_fw_activation_history(int argc, char **argv, struct command *cmd, struct plugin *plugin) @@ -2252,7 +2249,7 @@ static int micron_fw_activation_history(int argc, char **argv, struct command *c char formatted_output[100]; int count = 0; unsigned int logC2[C2_log_size/sizeof(int)] = { 0 }; - eDriveModel eModel = UNKNOWN_MODEL; + enum eDriveModel eModel = UNKNOWN_MODEL; struct nvme_dev *dev; struct format { char *fmt; @@ -2273,8 +2270,8 @@ static int micron_fw_activation_history(int argc, char **argv, struct command *c if (err < 0) return -1; - if (strcmp(cfg.fmt, "normal") != 0) { - fprintf (stderr, "only normal format is supported currently\n"); + if (strcmp(cfg.fmt, "normal")) { + fprintf(stderr, "only normal format is supported currently\n"); dev_close(dev); return -1; } @@ -2297,27 +2294,23 @@ static int micron_fw_activation_history(int argc, char **argv, struct command *c (struct micron_fw_activation_history_table *)logC2; /* check version and log page */ - if (table->log_page != 0xC2 || (table->version != 2 && table->version != 1)) - { + if (table->log_page != 0xC2 || (table->version != 2 && table->version != 1)) { fprintf(stderr, "Unsupported fw activation history page: %x, version: %x\n", table->log_page, table->version); goto out; } - if (table->num_entries == 0) { + if (!table->num_entries) { fprintf(stderr, "No entries were found in fw activation history log\n"); goto out; } - printf("%s", fw_activation_history_table_header); - for(count = 0; count < table->num_entries; count++) { + micron_fw_activation_history_header_print(); + for (count = 0; count < table->num_entries; count++) { memset(formatted_output, '\0', 100); - if (display_fw_activate_entry(count, - &table->entries[count], - formatted_output, NULL) == 0) - { + if (!display_fw_activate_entry(count, &table->entries[count], formatted_output, + NULL)) printf("%s\n", formatted_output); - } } out: dev_close(dev); @@ -2333,14 +2326,14 @@ static int micron_latency_stats_track(int argc, char **argv, struct command *cmd int err = 0; __u32 result = 0; const char *desc = "Enable, Disable or Get cmd latency monitoring stats"; - const char *option = "enable or disable or status, default is status"; - const char *command = "commands to monitor for - all|read|write|trim," - " default is all i.e, enabled for all commands"; - const char *thrtime = "The threshold value to use for latency monitoring in" - " milliseconds, default is 800ms"; + const char *option = "enable or disable or status, default is status"; + const char *command = + "commands to monitor for - all|read|write|trim, default is all i.e, enabled for all commands"; + const char *thrtime = + "The threshold value to use for latency monitoring in milliseconds, default is 800ms"; int fid = MICRON_FID_LATENCY_MONITOR; - eDriveModel model = UNKNOWN_MODEL; + enum eDriveModel model = UNKNOWN_MODEL; uint32_t command_mask = 0x7; /* 1:read 2:write 4:trim 7:all */ uint32_t timing_mask = 0x08080800; /* R[31-24]:W[23:16]:T[15:8]:0 */ uint32_t enable = 2; @@ -2392,32 +2385,29 @@ static int micron_latency_stats_track(int argc, char **argv, struct command *cmd }; err = nvme_get_features(&g_args); - if (err != 0) { + if (err) { printf("Failed to retrieve latency monitoring feature status\n"); dev_close(dev); - return err; + return err; } /* If it is to retrieve the status only */ if (enable == 2) { printf("Latency Tracking Statistics is currently %s", - (result & 0xFFFF0000) ? "enabled" : "disabled"); + (result & 0xFFFF0000) ? "enabled" : "disabled"); if ((result & 7) == 7) { printf(" for All commands\n"); - } else if ((result & 7) > 0) { + } else if ((result & 7) > 0) { printf(" for"); - if (result & 1) { + if (result & 1) printf(" Read"); - } - if (result & 2) { + if (result & 2) printf(" Write"); - } - if (result & 4) { + if (result & 4) printf(" Trim"); - } printf(" commands\n"); - } else if (result == 0) { - printf("\n"); + } else if (!result) { + printf("\n"); } dev_close(dev); return err; @@ -2429,14 +2419,13 @@ static int micron_latency_stats_track(int argc, char **argv, struct command *cmd printf("The maximum threshold value cannot be more than 2550 ms\n"); dev_close(dev); return -1; - } - /* timing mask is in terms of 10ms units, so min allowed is 10ms */ - else if ((opt.threshold % 10) != 0) { + } else if (opt.threshold % 10) { + /* timing mask is in terms of 10ms units, so min allowed is 10ms */ printf("The threshold value should be multiple of 10 ms\n"); dev_close(dev); return -1; - } - opt.threshold /= 10; + } + opt.threshold /= 10; } /* read-in command(s) to be monitored */ @@ -2458,7 +2447,7 @@ static int micron_latency_stats_track(int argc, char **argv, struct command *cmd struct nvme_set_features_args args = { .args_size = sizeof(args), - .fd = dev_fd(dev), + .fd = dev_fd(dev), .fid = MICRON_FID_LATENCY_MONITOR, .nsid = 0, .cdw11 = enable, @@ -2470,15 +2459,15 @@ static int micron_latency_stats_track(int argc, char **argv, struct command *cmd .data_len = 0, .data = NULL, .timeout = NVME_DEFAULT_IOCTL_TIMEOUT, - .result = &result, + .result = &result, }; err = nvme_set_features(&args); - if (err == 0) { + if (!err) { printf("Successfully %sd latency monitoring for %s commands with %dms threshold\n", - opt.option, opt.command, opt.threshold == 0 ? 800 : opt.threshold * 10); + opt.option, opt.command, !opt.threshold ? 800 : opt.threshold * 10); } else { printf("Failed to %s latency monitoring for %s commands with %dms threshold\n", - opt.option, opt.command, opt.threshold == 0 ? 800 : opt.threshold * 10); + opt.option, opt.command, !opt.threshold ? 800 : opt.threshold * 10); } dev_close(dev); @@ -2521,10 +2510,11 @@ static int micron_latency_stats_logs(int argc, char **argv, struct command *cmd, uint32_t dsm; uint32_t rfu[6]; } log[LATENCY_LOG_ENTRIES]; - eDriveModel model = UNKNOWN_MODEL; + enum eDriveModel model = UNKNOWN_MODEL; struct nvme_dev *dev; int err = -1; const char *desc = "Display Latency tracking log information"; + OPT_ARGS(opts) = { OPT_END() }; @@ -2541,15 +2531,14 @@ static int micron_latency_stats_logs(int argc, char **argv, struct command *cmd, return err; } /* print header and each log entry */ - printf("Timestamp, Latency, CmdTag, Opcode, Fuse, Psdt,Cid, Nsid," - "Slba_L, Slba_H, Nlb, DEAC, PRINFO, FUA,LR\n"); - for (int i = 0; i < LATENCY_LOG_ENTRIES; i++) { - printf("%"PRIu64",%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u\n", - log[i].timestamp,log[i].latency, log[i].cmdtag, log[i].opcode, - log[i].fuse, log[i].psdt, log[i].cid, log[i].nsid, - log[i].slba_low, log[i].slba_high, log[i].nlb, - log[i].deac, log[i].prinfo, log[i].fua, log[i].lr); - } + printf("Timestamp, Latency, CmdTag, Opcode, Fuse, Psdt, Cid, Nsid, Slba_L, Slba_H, Nlb, "); + printf("DEAC, PRINFO, FUA, LR\n"); + for (int i = 0; i < LATENCY_LOG_ENTRIES; i++) + printf("%"PRIu64",%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u\n", + log[i].timestamp, log[i].latency, log[i].cmdtag, log[i].opcode, + log[i].fuse, log[i].psdt, log[i].cid, log[i].nsid, + log[i].slba_low, log[i].slba_high, log[i].nlb, + log[i].deac, log[i].prinfo, log[i].fua, log[i].lr); printf("\n"); dev_close(dev); return err; @@ -2559,42 +2548,41 @@ static int micron_latency_stats_info(int argc, char **argv, struct command *cmd, struct plugin *plugin) { const char *desc = "display command latency statistics"; - const char *command = "command to display stats - all|read|write|trim" - "default is all"; + const char *command = "command to display stats - all|read|write|trimdefault is all"; int err = 0; struct nvme_dev *dev; - eDriveModel model = UNKNOWN_MODEL; + enum eDriveModel model = UNKNOWN_MODEL; #define LATENCY_BUCKET_COUNT 32 #define LATENCY_BUCKET_RSVD 32 struct micron_latency_stats { - uint64_t version; /* major << 32 | minior */ - uint64_t all_cmds[LATENCY_BUCKET_COUNT + LATENCY_BUCKET_RSVD]; - uint64_t read_cmds[LATENCY_BUCKET_COUNT + LATENCY_BUCKET_RSVD]; - uint64_t write_cmds[LATENCY_BUCKET_COUNT + LATENCY_BUCKET_RSVD]; - uint64_t trim_cmds[LATENCY_BUCKET_COUNT + LATENCY_BUCKET_RSVD]; - uint32_t reserved[255]; /* round up to 4K */ + uint64_t version; /* major << 32 | minior */ + uint64_t all_cmds[LATENCY_BUCKET_COUNT + LATENCY_BUCKET_RSVD]; + uint64_t read_cmds[LATENCY_BUCKET_COUNT + LATENCY_BUCKET_RSVD]; + uint64_t write_cmds[LATENCY_BUCKET_COUNT + LATENCY_BUCKET_RSVD]; + uint64_t trim_cmds[LATENCY_BUCKET_COUNT + LATENCY_BUCKET_RSVD]; + uint32_t reserved[255]; /* round up to 4K */ } log; struct latency_thresholds { - uint32_t start; - uint32_t end; - char *unit; + uint32_t start; + uint32_t end; + char *unit; } thresholds[LATENCY_BUCKET_COUNT] = { - {0, 50, "us"}, {50, 100, "us"}, {100, 150, "us"}, {150, 200, "us"}, - {200, 300, "us"}, {300, 400, "us"}, {400, 500, "us"}, {500, 600, "us"}, - {600, 700, "us"}, {700, 800, "us"}, {800, 900, "us"}, {900, 1000, "us"}, - {1, 5, "ms"}, {5, 10, "ms"}, {10, 20, "ms"}, {20, 50, "ms"}, {50, 100, "ms"}, - {100, 200, "ms"}, {200, 300, "ms"}, {300, 400, "ms"}, {400, 500, "ms"}, - {500, 600, "ms"}, {600, 700, "ms"}, {700, 800, "ms"}, {800, 900, "ms"}, - {900, 1000, "ms"}, {1, 2, "s"}, {2, 3, "s"}, {3, 4, "s"}, {4, 5, "s"}, - {5,8, "s"}, - {8, INT_MAX, "s"}, + {0, 50, "us"}, {50, 100, "us"}, {100, 150, "us"}, {150, 200, "us"}, + {200, 300, "us"}, {300, 400, "us"}, {400, 500, "us"}, {500, 600, "us"}, + {600, 700, "us"}, {700, 800, "us"}, {800, 900, "us"}, {900, 1000, "us"}, + {1, 5, "ms"}, {5, 10, "ms"}, {10, 20, "ms"}, {20, 50, "ms"}, {50, 100, "ms"}, + {100, 200, "ms"}, {200, 300, "ms"}, {300, 400, "ms"}, {400, 500, "ms"}, + {500, 600, "ms"}, {600, 700, "ms"}, {700, 800, "ms"}, {800, 900, "ms"}, + {900, 1000, "ms"}, {1, 2, "s"}, {2, 3, "s"}, {3, 4, "s"}, {4, 5, "s"}, + {5, 8, "s"}, + {8, INT_MAX, "s"}, }; struct { char *command; } opt = { - .command="all" + .command = "all" }; uint64_t *cmd_stats = &log.all_cmds[0]; @@ -2639,16 +2627,16 @@ static int micron_latency_stats_info(int argc, char **argv, struct command *cmd, printf("=============================================\n"); for (int b = 0; b < LATENCY_BUCKET_COUNT; b++) { - int bucket = b + 1; - char start[32] = { 0 }; - char end[32] = { 0 }; - sprintf(start, "%u%s", thresholds[b].start, thresholds[b].unit); - if (thresholds[b].end == INT_MAX) - sprintf(end, "INF"); - else - sprintf(end, "%u%s", thresholds[b].end, thresholds[b].unit); - printf("%2d %8s %8s %8"PRIu64"\n", - bucket, start, end, cmd_stats[b]); + int bucket = b + 1; + char start[32] = { 0 }; + char end[32] = { 0 }; + + sprintf(start, "%u%s", thresholds[b].start, thresholds[b].unit); + if (thresholds[b].end == INT_MAX) + sprintf(end, "INF"); + else + sprintf(end, "%u%s", thresholds[b].end, thresholds[b].unit); + printf("%2d %8s %8s %8"PRIu64"\n", bucket, start, end, cmd_stats[b]); } dev_close(dev); return err; @@ -2661,7 +2649,7 @@ static int micron_ocp_smart_health_logs(int argc, char **argv, struct command *c unsigned int logC0[C0_log_size/sizeof(int)] = { 0 }; unsigned int logFB[FB_log_size/sizeof(int)] = { 0 }; struct nvme_id_ctrl ctrl; - eDriveModel eModel = UNKNOWN_MODEL; + enum eDriveModel eModel = UNKNOWN_MODEL; struct nvme_dev *dev; bool is_json = true; struct format { @@ -2682,7 +2670,7 @@ static int micron_ocp_smart_health_logs(int argc, char **argv, struct command *c if (err < 0) return -1; - if (strcmp(cfg.fmt, "normal") == 0) + if (!strcmp(cfg.fmt, "normal")) is_json = false; /* For M5410 and M5407, this option prints 0xFB log page */ @@ -2690,9 +2678,9 @@ static int micron_ocp_smart_health_logs(int argc, char **argv, struct command *c __u8 spec = (eModel == M5410) ? 0 : 1; __u8 nsze; - if ((err = nvme_identify_ctrl(dev_fd(dev), &ctrl)) == 0) - err = nvme_get_log_simple(dev_fd(dev), 0xFB, - FB_log_size, logFB); + err = nvme_identify_ctrl(dev_fd(dev), &ctrl); + if (!err) + err = nvme_get_log_simple(dev_fd(dev), 0xFB, FB_log_size, logFB); if (err) { if (err < 0) printf("Unable to retrieve smart log 0xFB for the drive\n"); @@ -2700,7 +2688,7 @@ static int micron_ocp_smart_health_logs(int argc, char **argv, struct command *c } nsze = (ctrl.vs[987] == 0x12); - if (nsze == 0 && nsze_from_oacs) + if (!nsze && nsze_from_oacs) nsze = ((ctrl.oacs >> 3) & 0x1); print_nand_stats_fb((__u8 *)logFB, NULL, nsze, is_json, spec); goto out; @@ -2708,21 +2696,20 @@ static int micron_ocp_smart_health_logs(int argc, char **argv, struct command *c /* check for models that support 0xC0 log */ if (eModel != M51CX) { - printf ("Unsupported drive model for vs-smart-add-log commmand\n"); + printf("Unsupported drive model for vs-smart-add-log commmand\n"); err = -1; goto out; } err = nvme_get_log_simple(dev_fd(dev), 0xC0, C0_log_size, logC0); - if (err == 0) { + if (!err) print_smart_cloud_health_log((__u8 *)logC0, is_json); - } else if (err < 0) { + else if (err < 0) printf("Unable to retrieve extended smart log 0xC0 for the drive\n"); - } out: dev_close(dev); if (err > 0) - nvme_show_status(err); + nvme_show_status(err); return err; } @@ -2732,8 +2719,9 @@ static int micron_clr_fw_activation_history(int argc, char **argv, const char *desc = "Clear FW activation history"; __u32 result = 0; __u8 fid = MICRON_FEATURE_CLEAR_FW_ACTIVATION_HISTORY; - eDriveModel model = UNKNOWN_MODEL; + enum eDriveModel model = UNKNOWN_MODEL; struct nvme_dev *dev; + OPT_ARGS(opts) = { OPT_END() }; @@ -2744,14 +2732,16 @@ static int micron_clr_fw_activation_history(int argc, char **argv, return err; if (model != M51CX) { - printf ("This option is not supported for specified drive\n"); + printf("This option is not supported for specified drive\n"); dev_close(dev); return err; } err = nvme_set_features_simple(dev_fd(dev), fid, 1 << 31, 0, 0, &result); - if (err == 0) err = (int)result; - else printf ("Failed to clear fw activation history, error = 0x%x\n", err); + if (!err) + err = (int)result; + else + printf("Failed to clear fw activation history, error = 0x%x\n", err); dev_close(dev); return err; @@ -2764,12 +2754,11 @@ static int micron_telemetry_cntrl_option(int argc, char **argv, __u32 result = 0; const char *desc = "Enable or Disable Controller telemetry log generation"; const char *option = "enable or disable or status"; - const char *select = "select/save values: enable/disable options" - "1 - save (persistent), 0 - non-persistent and for " - "status options: 0 - current, 1 - default, 2-saved"; + const char *select = + "select/save values: enable/disable options1 - save (persistent), 0 - non-persistent and for status options: 0 - current, 1 - default, 2-saved"; int fid = MICRON_FEATURE_TELEMETRY_CONTROL_OPTION; - eDriveModel model = UNKNOWN_MODEL; - struct nvme_id_ctrl ctrl = { 0 }; + enum eDriveModel model = UNKNOWN_MODEL; + struct nvme_id_ctrl ctrl = { 0 }; struct nvme_dev *dev; struct { @@ -2777,7 +2766,7 @@ static int micron_telemetry_cntrl_option(int argc, char **argv, int select; } opt = { .option = "disable", - .select= 0, + .select = 0, }; OPT_ARGS(opts) = { @@ -2792,15 +2781,15 @@ static int micron_telemetry_cntrl_option(int argc, char **argv, err = nvme_identify_ctrl(dev_fd(dev), &ctrl); if ((ctrl.lpa & 0x8) != 0x8) { - printf("drive doesn't support host/controller generated telemetry logs\n"); - dev_close(dev); - return err; + printf("drive doesn't support host/controller generated telemetry logs\n"); + dev_close(dev); + return err; } if (!strcmp(opt.option, "enable")) { struct nvme_set_features_args args = { .args_size = sizeof(args), - .fd = dev_fd(dev), + .fd = dev_fd(dev), .fid = fid, .nsid = 1, .cdw11 = 1, @@ -2811,18 +2800,17 @@ static int micron_telemetry_cntrl_option(int argc, char **argv, .data_len = 0, .data = NULL, .timeout = NVME_DEFAULT_IOCTL_TIMEOUT, - .result = &result, + .result = &result, }; err = nvme_set_features(&args); - if (err == 0) { + if (!err) printf("successfully set controller telemetry option\n"); - } else { + else printf("Failed to set controller telemetry option\n"); - } } else if (!strcmp(opt.option, "disable")) { struct nvme_set_features_args args = { .args_size = sizeof(args), - .fd = dev_fd(dev), + .fd = dev_fd(dev), .fid = fid, .nsid = 1, .cdw11 = 0, @@ -2833,37 +2821,37 @@ static int micron_telemetry_cntrl_option(int argc, char **argv, .data_len = 0, .data = NULL, .timeout = NVME_DEFAULT_IOCTL_TIMEOUT, - .result = &result, + .result = &result, }; - err = nvme_set_features(&args); - if (err == 0) { + err = nvme_set_features(&args); + if (!err) printf("successfully disabled controller telemetry option\n"); - } else { + else printf("Failed to disable controller telemetry option\n"); - } } else if (!strcmp(opt.option, "status")) { - struct nvme_get_features_args args = { - .args_size = sizeof(args), - .fd = dev_fd(dev), - .fid = fid, - .nsid = 1, - .sel = opt.select & 0x3, - .cdw11 = 0, - .uuidx = 0, - .data_len = 0, - .data = NULL, - .timeout = NVME_DEFAULT_IOCTL_TIMEOUT, - .result = &result, - }; + struct nvme_get_features_args args = { + .args_size = sizeof(args), + .fd = dev_fd(dev), + .fid = fid, + .nsid = 1, + .sel = opt.select & 0x3, + .cdw11 = 0, + .uuidx = 0, + .data_len = 0, + .data = NULL, + .timeout = NVME_DEFAULT_IOCTL_TIMEOUT, + .result = &result, + }; + err = nvme_get_features(&args); - if (err == 0) { + if (!err) printf("Controller telemetry option : %s\n", - (result) ? "enabled" : "disabled"); - } else { + (result) ? "enabled" : "disabled"); + else printf("Failed to retrieve controller telemetry option\n"); - } } else { - printf("invalid option %s, valid values are enable,disable or status\n", opt.option); + printf("invalid option %s, valid values are enable,disable or status\n", + opt.option); dev_close(dev); return -1; } @@ -2940,83 +2928,81 @@ static int get_common_log(int fd, uint8_t id, uint8_t **buf, int *size) ret = nvme_get_log_simple(fd, id, sizeof(hdr), &hdr); if (ret) { - fprintf(stderr, "pull hdr failed for %hhu with error: 0x%x\n", id, ret); - return ret; + fprintf(stderr, "pull hdr failed for %u with error: 0x%x\n", id, ret); + return ret; } - if (hdr.id != id || - hdr.log_size == 0 || - hdr.max_size == 0 || - hdr.write_pointer < sizeof(hdr)) - { - fprintf(stderr, "invalid log data for LOG: 0x%X, id: 0x%X, size: %u, " - "max: %u, wp: %u, flags: %hhu, np: %u\n", id, - hdr.id, hdr.log_size, hdr.max_size, hdr.write_pointer, - hdr.flags, hdr.next_pointer); - return 1; + if (hdr.id != id || !hdr.log_size || !hdr.max_size || + hdr.write_pointer < sizeof(hdr)) { + fprintf(stderr, + "invalid log data for LOG: 0x%X, id: 0x%X, size: %u, max: %u, wp: %u, flags: %u, np: %u\n", + id, hdr.id, hdr.log_size, hdr.max_size, hdr.write_pointer, hdr.flags, + hdr.next_pointer); + return 1; } - /* we may have just 32-bytes for some models; write to wfile if log hasn't + /* + * we may have just 32-bytes for some models; write to wfile if log hasn't * yet reached its max size */ if (hdr.log_size == sizeof(hdr)) { buffer = (uint8_t *)malloc(sizeof(hdr)); - if (buffer == NULL) { + if (!buffer) { fprintf(stderr, "malloc of %zu bytes failed for log: 0x%X\n", sizeof(hdr), id); - return -ENOMEM; - } - memcpy(buffer,(uint8_t *)&hdr, sizeof(hdr)); + return -ENOMEM; + } + memcpy(buffer, (uint8_t *)&hdr, sizeof(hdr)); } else if (hdr.log_size < hdr.max_size) { - buffer = (uint8_t *)malloc(sizeof(hdr) + hdr.log_size); - if (buffer == NULL) { + buffer = (uint8_t *)malloc(sizeof(hdr) + hdr.log_size); + if (!buffer) { fprintf(stderr, "malloc of %zu bytes failed for log: 0x%X\n", hdr.log_size + sizeof(hdr), id); - return -ENOMEM; - } - memcpy(buffer, &hdr, sizeof(hdr)); + return -ENOMEM; + } + memcpy(buffer, &hdr, sizeof(hdr)); ret = nvme_get_log_lpo(fd, id, sizeof(hdr), chunk, hdr.log_size, - buffer + sizeof(hdr)); - if (ret == 0) { - log_size += hdr.log_size; - } + buffer + sizeof(hdr)); + if (!ret) + log_size += hdr.log_size; } else if (hdr.log_size >= hdr.max_size) { - /* reached maximum, to maintain, sequence we need to depend on write - * pointer to detect wrap-overs. FW doesn't yet implement the condition - * hdr.log_size > hdr.max_size; also ignore over-written log data; we - * also ignore collisions for now - */ - buffer = (uint8_t *)malloc(hdr.max_size + sizeof(hdr)); - if (buffer == NULL) { + /* + * reached maximum, to maintain, sequence we need to depend on write + * pointer to detect wrap-overs. FW doesn't yet implement the condition + * hdr.log_size > hdr.max_size; also ignore over-written log data; we + * also ignore collisions for now + */ + buffer = (uint8_t *)malloc(hdr.max_size + sizeof(hdr)); + if (!buffer) { fprintf(stderr, "malloc of %zu bytes failed for log: 0x%X\n", hdr.max_size + sizeof(hdr), id); - return -ENOMEM; - } - memcpy(buffer, &hdr, sizeof(hdr)); + return -ENOMEM; + } + memcpy(buffer, &hdr, sizeof(hdr)); first = hdr.max_size - hdr.write_pointer; second = hdr.write_pointer - sizeof(hdr); if (first) { ret = nvme_get_log_lpo(fd, id, hdr.write_pointer, chunk, first, - buffer + sizeof(hdr)); - if (ret) { + buffer + sizeof(hdr)); + if (ret) { free(buffer); - fprintf(stderr, "failed to get log: 0x%X\n", id); - return ret; + fprintf(stderr, "failed to get log: 0x%X\n", id); + return ret; + } + log_size += first; } - log_size += first; - } - if (second) { + if (second) { ret = nvme_get_log_lpo(fd, id, sizeof(hdr), chunk, second, - buffer + sizeof(hdr) + first); - if (ret) { - fprintf(stderr, "failed to get log: 0x%X\n", id); + buffer + sizeof(hdr) + first); + if (ret) { + fprintf(stderr, "failed to get log: 0x%X\n", id); free(buffer); - return ret; + return ret; + } + log_size += second; } - log_size += second; - } } *buf = buffer; *size = log_size; @@ -3090,7 +3076,7 @@ static int micron_internal_logs(int argc, char **argv, struct command *cmd, { 0xEA, "nvmelog_EA.bin", 0, 0 }, }; - eDriveModel eModel; + enum eDriveModel eModel; const char *desc = "This retrieves the micron debug log package"; const char *package = "Log output data file name (required)"; @@ -3126,36 +3112,38 @@ static int micron_internal_logs(int argc, char **argv, struct command *cmd, return err; /* if telemetry type is specified, check for data area */ - if (strlen(cfg.type) != 0) { + if (strlen(cfg.type)) { if (!strcmp(cfg.type, "controller")) { cfg.log = 0x08; } else if (strcmp(cfg.type, "host")) { - printf ("telemetry type (host or controller) should be specified i.e. -t=host\n"); + printf("telemetry type (host or controller) should be specified i.e. -t=host\n"); goto out; } if (cfg.data_area <= 0 || cfg.data_area > 3) { - printf ("data area must be selected using -d option ie --d=1,2,3\n"); + printf("data area must be selected using -d option ie --d=1,2,3\n"); goto out; } telemetry_option = 1; } else if (cfg.data_area > 0) { - printf ("data area option is valid only for telemetry option (i.e --type=host|controller)\n"); + printf("data area option is valid only for telemetry option (i.e --type=host|controller)\n"); goto out; } - if (strlen(cfg.package) == 0) { + if (!strlen(cfg.package)) { if (telemetry_option) - printf ("Log data file must be specified. ie -p=logfile.bin\n"); + printf("Log data file must be specified. ie -p=logfile.bin\n"); else - printf ("Log data file must be specified. ie -p=logfile.zip or -p=logfile.tgz|logfile.tar.gz\n"); + printf("Log data file must be specified. ie -p=logfile.zip or -p=logfile.tgz|logfile.tar.gz\n"); goto out; } /* pull log details based on the model name */ - sscanf(argv[optind], "/dev/nvme%d", &ctrlIdx); - if ((eModel = GetDriveModel(ctrlIdx)) == UNKNOWN_MODEL) { - printf ("Unsupported drive model for vs-internal-log collection\n"); + if (sscanf(argv[optind], "/dev/nvme%d", &ctrlIdx) != 1) + ctrlIdx = 0; + eModel = GetDriveModel(ctrlIdx); + if (eModel == UNKNOWN_MODEL) { + printf("Unsupported drive model for vs-internal-log collection\n"); goto out; } @@ -3166,13 +3154,14 @@ static int micron_internal_logs(int argc, char **argv, struct command *cmd, err = -EINVAL; if (telemetry_option) { if ((ctrl.lpa & 0x8) != 0x8) { - printf("telemetry option is not supported for specified drive\n"); - goto out; + printf("telemetry option is not supported for specified drive\n"); + goto out; } int logSize = 0; __u8 *buffer = NULL; const char *dir = "."; + err = micron_telemetry_log(dev_fd(dev), cfg.log, &buffer, &logSize, cfg.data_area); - if (err == 0 && logSize > 0 && buffer != NULL) { + if (!err && logSize > 0 && buffer) { sprintf(msg, "telemetry log: 0x%X", cfg.log); WriteData(buffer, logSize, dir, cfg.package, msg); free(buffer); @@ -3184,6 +3173,7 @@ static int micron_internal_logs(int argc, char **argv, struct command *cmd, /* trim spaces out of serial number string */ int i, j = 0; + for (i = 0; i < sizeof(ctrl.sn); i++) { if (isblank((int)ctrl.sn[i])) continue; @@ -3213,7 +3203,7 @@ static int micron_internal_logs(int argc, char **argv, struct command *cmd, if (eModel != M5410 && eModel != M5407) { memcpy(&aVendorLogs[c_logs_index], aM51XXLogs, sizeof(aM51XXLogs)); - c_logs_index += sizeof(aM51XXLogs)/sizeof(aM51XXLogs[0]); + c_logs_index += ARRAY_SIZE(aM51XXLogs); if (eModel == M51AX) memcpy((char *)&aVendorLogs[c_logs_index], aM51AXLogs, sizeof(aM51AXLogs)); else if (eModel == M51BX) @@ -3222,49 +3212,54 @@ static int micron_internal_logs(int argc, char **argv, struct command *cmd, memcpy((char *)&aVendorLogs[c_logs_index], aM51CXLogs, sizeof(aM51CXLogs)); } - for (int i = 0; i < (int)(sizeof(aVendorLogs) / sizeof(aVendorLogs[0])) && - aVendorLogs[i].ucLogPage != 0; i++) { + for (int i = 0; i < (int)(ARRAY_SIZE(aVendorLogs)) && aVendorLogs[i].ucLogPage; i++) { err = -1; switch (aVendorLogs[i].ucLogPage) { case 0xE1: + fallthrough; case 0xE5: + fallthrough; case 0xE9: err = 1; break; - case 0xE2: + fallthrough; case 0xE3: + fallthrough; case 0xE4: + fallthrough; case 0xE8: + fallthrough; case 0xEA: err = get_common_log(dev_fd(dev), aVendorLogs[i].ucLogPage, &dataBuffer, &bSize); break; - case 0xC1: + fallthrough; case 0xC2: + fallthrough; case 0xC4: err = GetLogPageSize(dev_fd(dev), aVendorLogs[i].ucLogPage, &bSize); - if (err == 0 && bSize > 0) + if (!err && bSize > 0) err = GetCommonLogPage(dev_fd(dev), aVendorLogs[i].ucLogPage, &dataBuffer, bSize); break; - case 0xE6: + fallthrough; case 0xE7: puiIDDBuf = (unsigned int *)&ctrl; uiMask = puiIDDBuf[1015]; - if (uiMask == 0 || (aVendorLogs[i].ucLogPage == 0xE6 && uiMask == 2) || - (aVendorLogs[i].ucLogPage == 0xE7 && uiMask == 1)) { + if (!uiMask || (aVendorLogs[i].ucLogPage == 0xE6 && uiMask == 2) || + (aVendorLogs[i].ucLogPage == 0xE7 && uiMask == 1)) { bSize = 0; } else { bSize = (int)puiIDDBuf[1023]; - if (bSize % (16 * 1024)) { + if (bSize % (16 * 1024)) bSize += (16 * 1024) - (bSize % (16 * 1024)); - } } - if (bSize != 0 && (dataBuffer = (unsigned char *)malloc(bSize)) != NULL) { + dataBuffer = (unsigned char *)malloc(bSize); + if (bSize && !dataBuffer) { memset(dataBuffer, 0, bSize); if (eModel == M5410 || eModel == M5407) err = NVMEGetLogPage(dev_fd(dev), @@ -3272,31 +3267,31 @@ static int micron_internal_logs(int argc, char **argv, struct command *cmd, bSize); else err = nvme_get_log_simple(dev_fd(dev), - aVendorLogs[i].ucLogPage, - bSize, dataBuffer); + aVendorLogs[i].ucLogPage, + bSize, dataBuffer); } break; - case 0xF7: + fallthrough; case 0xF9: + fallthrough; case 0xFC: + fallthrough; case 0xFD: - if (eModel == M51BX) { + if (eModel == M51BX) (void)NVMEResetLog(dev_fd(dev), aVendorLogs[i].ucLogPage, aVendorLogs[i].nLogSize, aVendorLogs[i].nMaxSize); - } - /* fallthrough */ + fallthrough; default: bSize = aVendorLogs[i].nLogSize; dataBuffer = (unsigned char *)malloc(bSize); - if (dataBuffer == NULL) { - break; - } + if (!dataBuffer) + break; memset(dataBuffer, 0, bSize); err = nvme_get_log_simple(dev_fd(dev), aVendorLogs[i].ucLogPage, bSize, dataBuffer); maxSize = aVendorLogs[i].nMaxSize - bSize; - while (err == 0 && maxSize > 0 && ((unsigned int *)dataBuffer)[0] != 0xdeadbeef) { + while (!err && maxSize > 0 && ((unsigned int *)dataBuffer)[0] != 0xdeadbeef) { sprintf(msg, "log 0x%x", aVendorLogs[i].ucLogPage); WriteData(dataBuffer, bSize, strCtrlDirName, aVendorLogs[i].strFileName, msg); err = nvme_get_log_simple(dev_fd(dev), @@ -3309,12 +3304,12 @@ static int micron_internal_logs(int argc, char **argv, struct command *cmd, break; } - if (err == 0 && dataBuffer != NULL && ((unsigned int *)dataBuffer)[0] != 0xdeadbeef) { + if (!err && dataBuffer && ((unsigned int *)dataBuffer)[0] != 0xdeadbeef) { sprintf(msg, "log 0x%x", aVendorLogs[i].ucLogPage); WriteData(dataBuffer, bSize, strCtrlDirName, aVendorLogs[i].strFileName, msg); } - if (dataBuffer != NULL) { + if (dataBuffer) { free(dataBuffer); dataBuffer = NULL; } @@ -3332,7 +3327,7 @@ static int micron_logpage_dir(int argc, char **argv, struct command *cmd, { int err = -1; const char *desc = "List the supported log pages"; - eDriveModel model = UNKNOWN_MODEL; + enum eDriveModel model = UNKNOWN_MODEL; char logbuf[MIN_LOG_SIZE]; struct nvme_dev *dev; int i; @@ -3382,10 +3377,11 @@ static int micron_logpage_dir(int argc, char **argv, struct command *cmd, }; printf("Supported log page list\nLog ID : Description\n"); - for (i = 0; i < sizeof(log_list)/sizeof(log_list[0]); i++) { + for (i = 0; i < ARRAY_SIZE(log_list); i++) { err = nvme_get_log_simple(dev_fd(dev), log_list[i].log_id, MIN_LOG_SIZE, &logbuf[0]); - if (err) continue; + if (err) + continue; printf("%02Xh : %s\n", log_list[i].log_id, log_list[i].desc); }