Skip to content

Commit 619453e

Browse files
walbournwalbourn
committed
More code review
1 parent af8ae93 commit 619453e

File tree

3 files changed

+91
-91
lines changed

3 files changed

+91
-91
lines changed

‎Texassemble/texassemble.cpp

+42-42
Original file line numberDiff line numberDiff line change
@@ -888,17 +888,6 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
888888
*pValue++ = 0;
889889

890890
dwOption = LookupByName(pArg, g_pOptionsLong);
891-
892-
if (dwOption == OPT_VERSION)
893-
{
894-
PrintLogo(true, g_ToolName, g_Description);
895-
return 0;
896-
}
897-
else if (dwOption == OPT_HELP)
898-
{
899-
PrintUsage();
900-
return 0;
901-
}
902891
}
903892
}
904893
else
@@ -922,19 +911,30 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
922911
}
923912
}
924913

925-
if (!dwOption)
914+
switch (dwOption)
926915
{
916+
case 0:
927917
wprintf(L"ERROR: Unknown option: `%ls`\n\nUse %ls --help\n", pArg, g_ToolName);
928918
return 1;
929-
}
930919

931-
if (dwOptions & (1 << dwOption))
932-
{
933-
wprintf(L"ERROR: Duplicate option: `%ls`\n\n", pArg);
934-
return 1;
935-
}
920+
case OPT_VERSION:
921+
PrintLogo(true, g_ToolName, g_Description);
922+
return 0;
923+
924+
case OPT_HELP:
925+
PrintUsage();
926+
return 0;
936927

937-
dwOptions |= 1 << dwOption;
928+
default:
929+
if (dwOptions & (1u << dwOption))
930+
{
931+
wprintf(L"ERROR: Duplicate option: `%ls`\n\n", pArg);
932+
return 1;
933+
}
934+
935+
dwOptions |= (1u << dwOption);
936+
break;
937+
}
938938

939939
// Handle options with additional value parameter
940940
switch (dwOption)
@@ -1154,7 +1154,7 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
11541154
{
11551155
const size_t count = conversion.size();
11561156
std::filesystem::path path(pArg);
1157-
SearchForFiles(path.make_preferred(), conversion, (dwOptions & (1 << OPT_RECURSIVE)) != 0, nullptr);
1157+
SearchForFiles(path.make_preferred(), conversion, (dwOptions & (1u << OPT_RECURSIVE)) != 0, nullptr);
11581158
if (conversion.size() <= count)
11591159
{
11601160
wprintf(L"No matching files found for %ls\n", pArg);
@@ -1176,7 +1176,7 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
11761176
return 0;
11771177
}
11781178

1179-
if (~dwOptions & (1 << OPT_NOLOGO))
1179+
if (~dwOptions & (1u << OPT_NOLOGO))
11801180
PrintLogo(false, g_ToolName, g_Description);
11811181

11821182
switch (dwCommand)
@@ -1231,7 +1231,7 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
12311231
outputFile = curpath.stem().concat(L".dds").native();
12321232
}
12331233

1234-
hr = LoadAnimatedGif(curpath.c_str(), loadedImages, (dwOptions & (1 << OPT_GIF_BGCOLOR)) != 0);
1234+
hr = LoadAnimatedGif(curpath.c_str(), loadedImages, (dwOptions & (1u << OPT_GIF_BGCOLOR)) != 0);
12351235
if (FAILED(hr))
12361236
{
12371237
wprintf(L" FAILED (%08X%ls)\n", static_cast<unsigned int>(hr), GetErrorDesc(hr));
@@ -1358,7 +1358,7 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
13581358
wprintf(L"\nERROR: Can't assemble complex surfaces\n");
13591359
return 1;
13601360
}
1361-
else if ((info.mipLevels > 1) && ((dwOptions & (1 << OPT_STRIP_MIPS)) == 0))
1361+
else if ((info.mipLevels > 1) && ((dwOptions & (1u << OPT_STRIP_MIPS)) == 0))
13621362
{
13631363
switch (dwCommand)
13641364
{
@@ -1538,7 +1538,7 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
15381538
}
15391539

15401540
// --- Strip Mips (if requested) -----------------------------------------------
1541-
if ((info.mipLevels > 1) && (dwOptions & (1 << OPT_STRIP_MIPS)))
1541+
if ((info.mipLevels > 1) && (dwOptions & (1u << OPT_STRIP_MIPS)))
15421542
{
15431543
std::unique_ptr<ScratchImage> timage(new (std::nothrow) ScratchImage);
15441544
if (!timage)
@@ -1588,7 +1588,7 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
15881588
}
15891589

15901590
// --- Undo Premultiplied Alpha (if requested) ---------------------------------
1591-
if ((dwOptions & (1 << OPT_DEMUL_ALPHA))
1591+
if ((dwOptions & (1u << OPT_DEMUL_ALPHA))
15921592
&& HasAlpha(info.format)
15931593
&& info.format != DXGI_FORMAT_A8_UNORM)
15941594
{
@@ -1695,7 +1695,7 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
16951695
}
16961696

16971697
// --- Tonemap (if requested) --------------------------------------------------
1698-
if (dwOptions & (1 << OPT_TONEMAP))
1698+
if (dwOptions & (1u << OPT_TONEMAP))
16991699
{
17001700
std::unique_ptr<ScratchImage> timage(new (std::nothrow) ScratchImage);
17011701
if (!timage)
@@ -2035,12 +2035,12 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
20352035
wprintf(L"\n");
20362036
fflush(stdout);
20372037

2038-
if (dwOptions & (1 << OPT_TOLOWER))
2038+
if (dwOptions & (1u << OPT_TOLOWER))
20392039
{
20402040
std::transform(outputFile.begin(), outputFile.end(), outputFile.begin(), towlower);
20412041
}
20422042

2043-
if (~dwOptions & (1 << OPT_OVERWRITE))
2043+
if (~dwOptions & (1u << OPT_OVERWRITE))
20442044
{
20452045
if (GetFileAttributesW(outputFile.c_str()) != INVALID_FILE_ATTRIBUTES)
20462046
{
@@ -2103,12 +2103,12 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
21032103
wprintf(L"\n");
21042104
fflush(stdout);
21052105

2106-
if (dwOptions & (1 << OPT_TOLOWER))
2106+
if (dwOptions & (1u << OPT_TOLOWER))
21072107
{
21082108
std::transform(outputFile.begin(), outputFile.end(), outputFile.begin(), towlower);
21092109
}
21102110

2111-
if (~dwOptions & (1 << OPT_OVERWRITE))
2111+
if (~dwOptions & (1u << OPT_OVERWRITE))
21122112
{
21132113
if (GetFileAttributesW(outputFile.c_str()) != INVALID_FILE_ATTRIBUTES)
21142114
{
@@ -2172,12 +2172,12 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
21722172
wprintf(L"\n");
21732173
fflush(stdout);
21742174

2175-
if (dwOptions & (1 << OPT_TOLOWER))
2175+
if (dwOptions & (1u << OPT_TOLOWER))
21762176
{
21772177
std::transform(outputFile.begin(), outputFile.end(), outputFile.begin(), towlower);
21782178
}
21792179

2180-
if (~dwOptions & (1 << OPT_OVERWRITE))
2180+
if (~dwOptions & (1u << OPT_OVERWRITE))
21812181
{
21822182
if (GetFileAttributesW(outputFile.c_str()) != INVALID_FILE_ATTRIBUTES)
21832183
{
@@ -2371,12 +2371,12 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
23712371
wprintf(L"\n");
23722372
fflush(stdout);
23732373

2374-
if (dwOptions & (1 << OPT_TOLOWER))
2374+
if (dwOptions & (1u << OPT_TOLOWER))
23752375
{
23762376
std::transform(outputFile.begin(), outputFile.end(), outputFile.begin(), towlower);
23772377
}
23782378

2379-
if (~dwOptions & (1 << OPT_OVERWRITE))
2379+
if (~dwOptions & (1u << OPT_OVERWRITE))
23802380
{
23812381
if (GetFileAttributesW(outputFile.c_str()) != INVALID_FILE_ATTRIBUTES)
23822382
{
@@ -2386,7 +2386,7 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
23862386
}
23872387

23882388
hr = SaveToDDSFile(result.GetImages(), result.GetImageCount(), result.GetMetadata(),
2389-
(dwOptions & (1 << OPT_USE_DX10)) ? (DDS_FLAGS_FORCE_DX10_EXT | DDS_FLAGS_FORCE_DX10_EXT_MISC2) : DDS_FLAGS_NONE,
2389+
(dwOptions & (1u << OPT_USE_DX10)) ? (DDS_FLAGS_FORCE_DX10_EXT | DDS_FLAGS_FORCE_DX10_EXT_MISC2) : DDS_FLAGS_NONE,
23902390
outputFile.c_str());
23912391
if (FAILED(hr))
23922392
{
@@ -2429,12 +2429,12 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
24292429
wprintf(L"\n");
24302430
fflush(stdout);
24312431

2432-
if (dwOptions & (1 << OPT_TOLOWER))
2432+
if (dwOptions & (1u << OPT_TOLOWER))
24332433
{
24342434
std::transform(outputFile.begin(), outputFile.end(), outputFile.begin(), towlower);
24352435
}
24362436

2437-
if (~dwOptions & (1 << OPT_OVERWRITE))
2437+
if (~dwOptions & (1u << OPT_OVERWRITE))
24382438
{
24392439
if (GetFileAttributesW(outputFile.c_str()) != INVALID_FILE_ATTRIBUTES)
24402440
{
@@ -2444,7 +2444,7 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
24442444
}
24452445

24462446
hr = SaveToDDSFile(result.GetImages(), result.GetImageCount(), result.GetMetadata(),
2447-
(dwOptions & (1 << OPT_USE_DX10)) ? (DDS_FLAGS_FORCE_DX10_EXT | DDS_FLAGS_FORCE_DX10_EXT_MISC2) : DDS_FLAGS_NONE,
2447+
(dwOptions & (1u << OPT_USE_DX10)) ? (DDS_FLAGS_FORCE_DX10_EXT | DDS_FLAGS_FORCE_DX10_EXT_MISC2) : DDS_FLAGS_NONE,
24482448
outputFile.c_str());
24492449
if (FAILED(hr))
24502450
{
@@ -2518,7 +2518,7 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
25182518

25192519
case CMD_ARRAY:
25202520
case CMD_GIF:
2521-
hr = result.InitializeArrayFromImages(&imageArray[0], imageArray.size(), (dwOptions & (1 << OPT_USE_DX10)) != 0);
2521+
hr = result.InitializeArrayFromImages(&imageArray[0], imageArray.size(), (dwOptions & (1u << OPT_USE_DX10)) != 0);
25222522
break;
25232523

25242524
case CMD_CUBE:
@@ -2542,12 +2542,12 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
25422542
wprintf(L"\n");
25432543
fflush(stdout);
25442544

2545-
if (dwOptions & (1 << OPT_TOLOWER))
2545+
if (dwOptions & (1u << OPT_TOLOWER))
25462546
{
25472547
std::transform(outputFile.begin(), outputFile.end(), outputFile.begin(), towlower);
25482548
}
25492549

2550-
if (~dwOptions & (1 << OPT_OVERWRITE))
2550+
if (~dwOptions & (1u << OPT_OVERWRITE))
25512551
{
25522552
if (GetFileAttributesW(outputFile.c_str()) != INVALID_FILE_ATTRIBUTES)
25532553
{
@@ -2557,7 +2557,7 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
25572557
}
25582558

25592559
hr = SaveToDDSFile(result.GetImages(), result.GetImageCount(), result.GetMetadata(),
2560-
(dwOptions & (1 << OPT_USE_DX10)) ? (DDS_FLAGS_FORCE_DX10_EXT | DDS_FLAGS_FORCE_DX10_EXT_MISC2) : DDS_FLAGS_NONE,
2560+
(dwOptions & (1u << OPT_USE_DX10)) ? (DDS_FLAGS_FORCE_DX10_EXT | DDS_FLAGS_FORCE_DX10_EXT_MISC2) : DDS_FLAGS_NONE,
25612561
outputFile.c_str());
25622562
if (FAILED(hr))
25632563
{

‎Texconv/texconv.cpp

+19-19
Original file line numberDiff line numberDiff line change
@@ -1315,17 +1315,6 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
13151315
*pValue++ = 0;
13161316

13171317
dwOption = LookupByName(pArg, g_pOptionsLong);
1318-
1319-
if (dwOption == OPT_VERSION)
1320-
{
1321-
PrintLogo(true, g_ToolName, g_Description);
1322-
return 0;
1323-
}
1324-
else if (dwOption == OPT_HELP)
1325-
{
1326-
PrintUsage();
1327-
return 0;
1328-
}
13291318
}
13301319
}
13311320
else
@@ -1349,19 +1338,30 @@ int __cdecl wmain(_In_ int argc, _In_z_count_(argc) wchar_t* argv[])
13491338
}
13501339
}
13511340

1352-
if (!dwOption)
1341+
switch (dwOption)
13531342
{
1343+
case 0:
13541344
wprintf(L"ERROR: Unknown option: `%ls`\n\nUse %ls --help\n", pArg, g_ToolName);
13551345
return 1;
1356-
}
13571346

1358-
if (dwOptions & (uint64_t(1) << dwOption))
1359-
{
1360-
wprintf(L"ERROR: Duplicate option: `%ls`\n\n", pArg);
1361-
return 1;
1362-
}
1347+
case OPT_VERSION:
1348+
PrintLogo(true, g_ToolName, g_Description);
1349+
return 0;
1350+
1351+
case OPT_HELP:
1352+
PrintUsage();
1353+
return 0;
13631354

1364-
dwOptions |= (uint64_t(1) << dwOption);
1355+
default:
1356+
if (dwOptions & (uint64_t(1) << dwOption))
1357+
{
1358+
wprintf(L"ERROR: Duplicate option: `%ls`\n\n", pArg);
1359+
return 1;
1360+
}
1361+
1362+
dwOptions |= (uint64_t(1) << dwOption);
1363+
break;
1364+
}
13651365

13661366
// Handle options with additional value parameter
13671367
switch (dwOption)

0 commit comments

Comments
 (0)