Skip to content

Commit

Permalink
feat: Adding in more clang-tidy check and resolving the issues
Browse files Browse the repository at this point in the history
Added in more clang-tidy for readability and misc checks.
Not all are added for how many warnings there are for some checks so handling them one at a time.

While fixing the various issues I also implemented safe_strto(u)l(l) functions to have more consistent error checks everywhere we use them to parse strings.

Signed-off-by: Tyler Erickson <tyler.erickson@seagate.com>
  • Loading branch information
vonericsen committed Nov 18, 2024
1 parent c6d9f9a commit 87b5dc5
Show file tree
Hide file tree
Showing 13 changed files with 172 additions and 100 deletions.
44 changes: 35 additions & 9 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -1,12 +1,38 @@
Checks: '-*,modernize-*,cert-*,bugprone-*,-bugprone-easily-swappable-parameters,clang-analyzer-*,-clang-analyzer-deadcode.DeadStores'
#other available checks: clang-analyzer-*, performance-*, readability-*, misc-*
#Disabled checks:
# -bugprone-easily-swappable-parameters - warns a LOT. Sometimes on functions meant to look like standardized C11 annex k functions
# While a overall useful thing to consider, it's too noisy right now and it may make the API
# harder to use to resolve all of these.
# -clang-analyzer-deadcode.DeadStores - A lot of warnings related to this. Will be good to cleanup, but due to how many
# there currently are, it's better to concentrate on other more pressing problems
Checks:
'-*,
modernize-*,
cert-*,
bugprone-*,
-bugprone-easily-swappable-parameters,
clang-analyzer-*,
-clang-analyzer-deadcode.DeadStores,
misc-*,
-misc-no-recursion,
-misc-unused-parameters,
readability-non-const-parameter,
readability-inconsistent-declaration-parameter-name,
readability-redundant-control-flow,
readability-duplicate-include,
readability-avoid-const-params-in-decls,
readability-function-cognitive-complexity'

# Other available checks:
# clang-analyzer-*, performance-*, readability-*, misc-*

# Disabled checks:
# - bugprone-easily-swappable-parameters:
# Warns a LOT. Sometimes on functions meant to look like standardized C11 annex k functions.
# While overall useful, it's too noisy right now and may complicate API usability.
# - clang-analyzer-deadcode.DeadStores:
# Generates many warnings. Cleanup is needed, but focus on more pressing issues first.
# - misc-no-recursion:
# Recursion is useful in our code, so this check is not applicable.
# - misc-unused-parameters:
# Too many false positives.
# - readability-*:
# Currently generates too many warnings. Manually adding rules until we can address these issues later.

WarningsAsErrors: ''
HeaderFilterRegex: '.*'
AnalyzeTemporaryDtors: false
FormatStyle: 'file'
FormatStyle: 'file'
10 changes: 5 additions & 5 deletions include/openseachest_util_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -4521,12 +4521,12 @@ extern "C"
//! \brief Description: This function prints out the short or long help for the fast discovery option
//
// Entry:
//! \param[in] shortUsage = bool used to select when to print short or long help
//! \param[in] shortHelp = bool used to select when to print short or long help
//
// Exit:
//
//-----------------------------------------------------------------------------
void print_Fast_Discovery_Help(bool shortUsage);
void print_Fast_Discovery_Help(bool shortHelp);

void print_Firmware_Download_Help(bool shortHelp);

Expand Down Expand Up @@ -4880,9 +4880,9 @@ extern "C"
//-----------------------------------------------------------------------------
void print_FARM_Log_Help(bool shortHelp);

void print_FARM_Combined_Log_Help(bool shortUsage);
void print_FARM_Combined_Log_Help(bool shortHelp);

void print_Sata_FARM_Copy_Type_Flag_Help(bool shortUsage);
void print_Sata_FARM_Copy_Type_Flag_Help(bool shortHelp);

void print_Show_SMART_Error_Log_Help(bool shortHelp);

Expand Down Expand Up @@ -4946,7 +4946,7 @@ extern "C"

void print_No_Time_Limit_Help(bool shortHelp);

void print_No_Banner_Help(bool shortUsage);
void print_No_Banner_Help(bool shortHelp);

void print_SAS_Phy_Partial_Help(bool shortHelp);

Expand Down
6 changes: 0 additions & 6 deletions src/EULA.c
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,6 @@ void print_EULA_To_Screen(void)
printf(" This Source Code Form is \"Incompatible With Secondary Licenses\", as\n");
printf(" defined by the Mozilla Public License, v. 2.0.\n\n");
print_Open_Source_Licenses();
return;
}

static void print_Win_Getopt_Licenses(void)
Expand Down Expand Up @@ -463,7 +462,6 @@ static void print_Win_Getopt_Licenses(void)
printf("ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT\n");
printf("(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS\n");
printf("SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.\n\n");
return;
}

#if defined(__FreeBSD__)
Expand Down Expand Up @@ -655,7 +653,6 @@ static void print_GNU_LGPL_License(void)
printf("future versions of the GNU Lesser General Public License shall apply, that\n");
printf("proxy's public statement of acceptance of any version is permanent\n");
printf("authorization for you to choose that version for the Library.\n\n");
return;
}
#endif //__GLIBC__

Expand Down Expand Up @@ -684,7 +681,6 @@ static void print_Musl_MIT_License(void)
"CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,\n"
"TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE\n"
"SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.\n\n");
return;
}
#endif // USING_MUSL_LIBC

Expand Down Expand Up @@ -734,7 +730,6 @@ static void print_Open_Fabrics_NVMe_IOCTL_License(void)
printf("official policies, either expressed or implied, of Intel Corporation, \n");
printf("Integrated Device Technology Inc., or Sandforce Corporation. \n");
printf("\n");
return;
}
#endif //_WIN32

Expand Down Expand Up @@ -769,5 +764,4 @@ void print_Open_Source_Licenses(void)
# error Please update #if for system library licenses!
#endif
printf("===========================================================================\n\n");
return;
}
58 changes: 30 additions & 28 deletions src/openseachest_util_options.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,10 @@ M_NODISCARD bool set_Verbosity_From_String(const char* requestedLevel, eVerbosit
bool set = false;
if (requestedLevel && verbosity)
{
char* end = M_NULLPTR;
long temp = strtol(requestedLevel, &end, 10);
if (!(temp == LONG_MAX && errno == ERANGE) && !(temp == 0 && requestedLevel == end) && strcmp(end, "") == 0 &&
C_CAST(eVerbosityLevels, temp) < VERBOSITY_MAX)
long temp = 0L;
if (0 == safe_strtol(&temp, requestedLevel, M_NULLPTR, BASE_10_DECIMAL))
{
*verbosity = C_CAST(eVerbosityLevels, temp);
*verbosity = M_STATIC_CAST(eVerbosityLevels, temp);
set = true;
}
}
Expand Down Expand Up @@ -135,7 +133,6 @@ void print_Elevated_Privileges_Text(void)
printf(
"In Linux/Unix, log in to a root terminal (su), then execute the command. This requires the root password.\n");
#endif
return;
}

char* get_current_year(char* temp_year)
Expand All @@ -149,13 +146,12 @@ char* get_current_year(char* temp_year)
return temp_year;
}

#include "common_types.h"

void openseachest_utility_Info(const char* utilityName, const char* buildVersion, const char* seaCPublicVersion)
{
eArchitecture architecture = get_Compiled_Architecture();
char* year = safe_calloc(CURRENT_YEAR_LENGTH, sizeof(char));
char* userName = M_NULLPTR;
errno_t userdup = 0;
#if defined(ENABLE_READ_USERNAME)
if (SUCCESS != get_Current_User_Name(&userName))
{
Expand All @@ -165,22 +161,25 @@ void openseachest_utility_Info(const char* utilityName, const char* buildVersion
{
snprintf(userName, UNKNOWN_USER_NAME_MAX_LENGTH, "Unable to retrieve current username");
}
else
{
userdup = ENOMEM;
}
}
#else //! ENABLE_READ_USERNAME
if (is_Running_Elevated())
{
# if defined(_WIN32)
userName = strdup("admin");
userdup = safe_strdup(&userName, "admin");
# else //!_WIN32
userName = strdup("root");
userdup = safe_strdup(&userName, "root");
# endif //_WIN32
}
else
{
userName = strdup("current user");
userdup = safe_strdup(&userName, "current user");
}
#endif // ENABLE_READ_USERNAME
// DECLARE_ZERO_INIT_ARRAY(char, g_timeString, 64);
printf("==========================================================================================\n");
printf(" %s - openSeaChest drive utilities", utilityName);
printf(" - NVMe Enabled");
Expand All @@ -194,8 +193,12 @@ void openseachest_utility_Info(const char* utilityName, const char* buildVersion
{
snprintf_err_handle(CURRENT_TIME_STRING, CURRENT_TIME_STRING_LENGTH, "Unable to get local time");
}
printf(" Today: %s\tUser: %s\n", CURRENT_TIME_STRING, userName);
printf("==========================================================================================\n");
printf(" Today: %s", CURRENT_TIME_STRING);
if (userdup == 0)
{
printf("\tUser: %s", userName);
}
printf("\n==========================================================================================\n");
safe_free(&userName);
safe_free(&year);
}
Expand Down Expand Up @@ -283,59 +286,58 @@ void print_SeaChest_Util_Exit_Codes(int numberOfToolSpecificE

void get_Scan_Flags(deviceScanFlags* scanFlags, char* optarg)
{
if (strncmp("ata", optarg, safe_strlen(optarg)) == 0)
if (strcmp("ata", optarg) == 0)
{
scanFlags->scanATA = true;
}
else if (safe_strlen(optarg) == 3 && strncmp("usb", optarg, safe_strlen(optarg)) == 0)
else if (strcmp("usb", optarg) == 0)
{
scanFlags->scanUSB = true;
}
else if (safe_strlen(optarg) == 4 && strncmp("scsi", optarg, safe_strlen(optarg)) == 0)
else if (strcmp("scsi", optarg) == 0)
{
scanFlags->scanSCSI = true;
}
else if (safe_strlen(optarg) == 4 && strncmp("nvme", optarg, safe_strlen(optarg)) == 0)
else if (strcmp("nvme", optarg) == 0)
{
scanFlags->scanNVMe = true;
}
else if (safe_strlen(optarg) == 4 && strncmp("raid", optarg, safe_strlen(optarg)) == 0)
else if (strcmp("raid", optarg) == 0)
{
scanFlags->scanRAID = true;
}
else if (safe_strlen(optarg) == 12 && strncmp("interfaceATA", optarg, safe_strlen(optarg)) == 0)
else if (strcmp("interfaceATA", optarg) == 0)
{
scanFlags->scanInterfaceATA = true;
}
else if (safe_strlen(optarg) == 12 && strncmp("interfaceUSB", optarg, safe_strlen(optarg)) == 0)
else if (strcmp("interfaceUSB", optarg) == 0)
{
scanFlags->scanInterfaceUSB = true;
}
else if (safe_strlen(optarg) == 13 && strncmp("interfaceSCSI", optarg, safe_strlen(optarg)) == 0)
else if (strcmp("interfaceSCSI", optarg) == 0)
{
scanFlags->scanInterfaceSCSI = true;
}
else if (safe_strlen(optarg) == 13 && strncmp("interfaceNVME", optarg, safe_strlen(optarg)) == 0)
else if (strcmp("interfaceNVME", optarg) == 0)
{
scanFlags->scanInterfaceNVMe = true;
}
else if (safe_strlen(optarg) == 2 && strncmp("sd", optarg, safe_strlen(optarg)) == 0)
else if (strcmp("sd", optarg) == 0)
{
scanFlags->scanSD = true;
}
else if (safe_strlen(optarg) == 6 && strncmp("sgtosd", optarg, safe_strlen(optarg)) == 0)
else if (strcmp("sgtosd", optarg) == 0)
{
scanFlags->scanSDandSG = true;
}
else if (safe_strlen(optarg) == 10 && strncmp("ignoreCSMI", optarg, safe_strlen(optarg)) == 0)
else if (strcmp("ignoreCSMI", optarg) == 0)
{
scanFlags->scanIgnoreCSMI = true;
}
else if (safe_strlen(optarg) == 15 && strncmp("allowDuplicates", optarg, safe_strlen(optarg)) == 0)
else if (strcmp("allowDuplicates", optarg) == 0)
{
scanFlags->scanAllowDuplicateDevices = true;
}
return;
}

void print_Scan_Help(bool shortHelp, const char* helpdeviceHandleExample)
Expand Down
2 changes: 1 addition & 1 deletion subprojects/wingetopt
Submodule wingetopt updated 1 files
+10 −0 src/getopt.c
Loading

0 comments on commit 87b5dc5

Please sign in to comment.