Code cleanup.

Fix improper test of return value, replace order-dependent indices with
enumerated types in option parsing.

Review URL: http://codereview.chromium.org/2183001
This commit is contained in:
Bill Richardson
2010-05-25 10:48:39 -07:00
parent 590d10b109
commit 93b09e5a4d

View File

@@ -80,96 +80,108 @@ void KernelUtility::PrintUsage(void) {
} }
bool KernelUtility::ParseCmdLineOptions(int argc, char* argv[]) { bool KernelUtility::ParseCmdLineOptions(int argc, char* argv[]) {
int option_index; int option_index, i;
enum {
OPT_FIRMWARE_KEY = 1000,
OPT_FIRMWARE_KEY_PUB,
OPT_KERNEL_KEY,
OPT_KERNEL_KEY_PUB,
OPT_FIRMWARE_SIGN_ALGORITHM,
OPT_KERNEL_SIGN_ALGORITHM,
OPT_KERNEL_KEY_VERSION,
OPT_KERNEL_VERSION,
OPT_IN,
OPT_OUT,
OPT_GENERATE,
OPT_VERIFY,
OPT_DESCRIBE,
OPT_CONFIG,
OPT_VBLOCK,
};
static struct option long_options[] = { static struct option long_options[] = {
{"firmware_key", 1, 0, 0}, {"firmware_key", 1, 0, OPT_FIRMWARE_KEY },
{"firmware_key_pub", 1, 0, 0}, {"firmware_key_pub", 1, 0, OPT_FIRMWARE_KEY_PUB },
{"kernel_key", 1, 0, 0}, {"kernel_key", 1, 0, OPT_KERNEL_KEY },
{"kernel_key_pub", 1, 0, 0}, {"kernel_key_pub", 1, 0, OPT_KERNEL_KEY_PUB },
{"firmware_sign_algorithm", 1, 0, 0}, {"firmware_sign_algorithm", 1, 0, OPT_FIRMWARE_SIGN_ALGORITHM },
{"kernel_sign_algorithm", 1, 0, 0}, {"kernel_sign_algorithm", 1, 0, OPT_KERNEL_SIGN_ALGORITHM },
{"kernel_key_version", 1, 0, 0}, {"kernel_key_version", 1, 0, OPT_KERNEL_KEY_VERSION },
{"kernel_version", 1, 0, 0}, {"kernel_version", 1, 0, OPT_KERNEL_VERSION },
{"in", 1, 0, 0}, {"in", 1, 0, OPT_IN },
{"out", 1, 0, 0}, {"out", 1, 0, OPT_OUT },
{"generate", 0, 0, 0}, {"generate", 0, 0, OPT_GENERATE },
{"verify", 0, 0, 0}, {"verify", 0, 0, OPT_VERIFY },
{"describe", 0, 0, 0}, {"describe", 0, 0, OPT_DESCRIBE },
{"config", 1, 0, 0}, {"config", 1, 0, OPT_CONFIG },
{"vblock", 0, 0, 0}, {"vblock", 0, 0, OPT_VBLOCK },
{NULL, 0, 0, 0} {NULL, 0, 0, 0}
}; };
while (1) { while ((i = getopt_long(argc, argv, "", long_options, &option_index)) != -1) {
int i = getopt_long(argc, argv, "", long_options, &option_index); switch (i) {
if (-1 == i) // Done with option processing. case '?':
break;
if ('?' == i) // Invalid option found.
return false; return false;
break;
if (0 == i) { case OPT_FIRMWARE_KEY:
switch (option_index) { firmware_key_file_ = optarg;
case 0: // firmware_key break;
firmware_key_file_ = optarg; case OPT_FIRMWARE_KEY_PUB:
break; firmware_key_pub_file_ = optarg;
case 1: // firmware_key_pub break;
firmware_key_pub_file_ = optarg; case OPT_KERNEL_KEY:
break; kernel_key_file_ = optarg;
case 2: // kernel_key break;
kernel_key_file_ = optarg; case OPT_KERNEL_KEY_PUB:
break; kernel_key_pub_file_ = optarg;
case 3: // kernel_key_pub break;
kernel_key_pub_file_ = optarg; case OPT_FIRMWARE_SIGN_ALGORITHM:
break; errno = 0; // strtol() returns an error via errno
case 4: // firmware_sign_algorithm firmware_sign_algorithm_ = strtol(optarg,
errno = 0; // strtol() returns an error via errno reinterpret_cast<char**>(NULL), 10);
firmware_sign_algorithm_ = strtol(optarg, if (errno)
reinterpret_cast<char**>(NULL), 10); return false;
if (errno) break;
return false; case OPT_KERNEL_SIGN_ALGORITHM:
break; errno = 0;
case 5: // kernel_sign_algorithm kernel_sign_algorithm_ = strtol(optarg,
errno = 0; reinterpret_cast<char**>(NULL), 10);
kernel_sign_algorithm_ = strtol(optarg, if (errno)
reinterpret_cast<char**>(NULL), 10); return false;
if (errno) break;
return false; case OPT_KERNEL_KEY_VERSION:
break; errno = 0;
case 6: // kernel_key_version kernel_key_version_ = strtol(optarg,
errno = 0;
kernel_key_version_ = strtol(optarg,
reinterpret_cast<char**>(NULL), 10);
if (errno)
return false;
break;
case 7: // kernel_version
errno = 0;
kernel_version_ = strtol(optarg,
reinterpret_cast<char**>(NULL), 10); reinterpret_cast<char**>(NULL), 10);
if (errno) if (errno)
return false; return false;
break; break;
case 8: // in case OPT_KERNEL_VERSION:
in_file_ = optarg; errno = 0;
break; kernel_version_ = strtol(optarg,
case 9: // out reinterpret_cast<char**>(NULL), 10);
out_file_ = optarg; if (errno)
break; return false;
case 10: // generate break;
is_generate_ = true; case OPT_IN:
break; in_file_ = optarg;
case 11: // verify break;
is_verify_ = true; case OPT_OUT:
break; out_file_ = optarg;
case 12: // describe break;
is_describe_ = true; case OPT_GENERATE:
break; is_generate_ = true;
case 13: // config break;
config_file_ = optarg; case OPT_VERIFY:
break; is_verify_ = true;
case 14: // vblock break;
is_only_vblock_ = true; case OPT_DESCRIBE:
break; is_describe_ = true;
} break;
case OPT_CONFIG:
config_file_ = optarg;
break;
case OPT_VBLOCK:
is_only_vblock_ = true;
break;
} }
} }
return CheckOptions(); return CheckOptions();
@@ -232,7 +244,7 @@ bool KernelUtility::GenerateSignedImage(void) {
} }
image_->kernel_data = BufferFromFile(in_file_.c_str(), image_->kernel_data = BufferFromFile(in_file_.c_str(),
&image_->kernel_len); &image_->kernel_len);
if (!image_) if (!image_->kernel_data)
return false; return false;
// Generate and add the signatures. // Generate and add the signatures.
if (!AddKernelKeySignature(image_, firmware_key_file_.c_str())) { if (!AddKernelKeySignature(image_, firmware_key_file_.c_str())) {