diff mbox

[v3,3/4] external: simplify help output in xscom commands

Message ID 1457526636-3634-4-git-send-email-clg@fr.ibm.com
State Accepted
Headers show

Commit Message

Cédric Le Goater March 9, 2016, 12:30 p.m. UTC
This kills a few booleans used to output version and help.

Suggested-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
 external/xscom-utils/getscom.c | 19 ++++++-------------
 external/xscom-utils/getsram.c | 22 ++++++----------------
 external/xscom-utils/putscom.c | 23 +++++++----------------
 3 files changed, 19 insertions(+), 45 deletions(-)

Comments

Vasant Hegde March 10, 2016, 10:33 a.m. UTC | #1
On 03/09/2016 06:00 PM, Cédric Le Goater wrote:
> This kills a few booleans used to output version and help.
>
> Suggested-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
> ---
>   external/xscom-utils/getscom.c | 19 ++++++-------------
>   external/xscom-utils/getsram.c | 22 ++++++----------------
>   external/xscom-utils/putscom.c | 23 +++++++----------------
>   3 files changed, 19 insertions(+), 45 deletions(-)
>
> diff --git a/external/xscom-utils/getscom.c b/external/xscom-utils/getscom.c
> index 45bdf2c2df92..96dcace433cd 100644
> --- a/external/xscom-utils/getscom.c
> +++ b/external/xscom-utils/getscom.c
> @@ -28,6 +28,7 @@ static void print_usage(void)
>   	printf("usage: getscom [-c|--chip chip-id] addr\n");
>   	printf("       getscom -l|--list-chips\n");
>   	printf("       getscom -v|--version\n");
> +	exit(1);

So its doing what is expected (display usage and exit). Better exit(0) ?


>   }
>
>   static void print_chip_info(uint32_t chip_id)
> @@ -81,9 +82,7 @@ int main(int argc, char *argv[])
>   {
>   	uint64_t val, addr = -1ull;
>   	uint32_t def_chip, chip_id = 0xffffffff;
> -	bool show_help = false;
>   	bool list_chips = false;
> -	bool show_version = false;
>   	bool no_work = false;
>   	int rc;
>
> @@ -107,14 +106,14 @@ int main(int argc, char *argv[])
>   			chip_id = strtoul(optarg, NULL, 0);
>   			break;
>   		case 'h':
> -			show_help = true;
> +			print_usage();
>   			break;
>   		case 'l':
>   			list_chips = true;
>   			break;
>   		case 'v':
> -			show_version = true;
> -			break;
> +			printf("xscom utils version %s\n", VERSION_STR);
> +			exit(1);

exit(0)?

-Vasant
Cédric Le Goater March 10, 2016, 10:54 a.m. UTC | #2
On 03/10/2016 11:33 AM, Vasant Hegde wrote:
> On 03/09/2016 06:00 PM, Cédric Le Goater wrote:
>> This kills a few booleans used to output version and help.
>>
>> Suggested-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
>> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
>> ---
>>   external/xscom-utils/getscom.c | 19 ++++++-------------
>>   external/xscom-utils/getsram.c | 22 ++++++----------------
>>   external/xscom-utils/putscom.c | 23 +++++++----------------
>>   3 files changed, 19 insertions(+), 45 deletions(-)
>>
>> diff --git a/external/xscom-utils/getscom.c b/external/xscom-utils/getscom.c
>> index 45bdf2c2df92..96dcace433cd 100644
>> --- a/external/xscom-utils/getscom.c
>> +++ b/external/xscom-utils/getscom.c
>> @@ -28,6 +28,7 @@ static void print_usage(void)
>>       printf("usage: getscom [-c|--chip chip-id] addr\n");
>>       printf("       getscom -l|--list-chips\n");
>>       printf("       getscom -v|--version\n");
>> +    exit(1);
> 
> So its doing what is expected (display usage and exit). Better exit(0) ?
> 
> 
>>   }
>>
>>   static void print_chip_info(uint32_t chip_id)
>> @@ -81,9 +82,7 @@ int main(int argc, char *argv[])
>>   {
>>       uint64_t val, addr = -1ull;
>>       uint32_t def_chip, chip_id = 0xffffffff;
>> -    bool show_help = false;
>>       bool list_chips = false;
>> -    bool show_version = false;
>>       bool no_work = false;
>>       int rc;
>>
>> @@ -107,14 +106,14 @@ int main(int argc, char *argv[])
>>               chip_id = strtoul(optarg, NULL, 0);
>>               break;
>>           case 'h':
>> -            show_help = true;
>> +            print_usage();
>>               break;
>>           case 'l':
>>               list_chips = true;
>>               break;
>>           case 'v':
>> -            show_version = true;
>> -            break;
>> +            printf("xscom utils version %s\n", VERSION_STR);
>> +            exit(1);
> 
> exit(0)?

yes. the patch breaks the exit code with this shortcut.

Thanks,


C.
diff mbox

Patch

diff --git a/external/xscom-utils/getscom.c b/external/xscom-utils/getscom.c
index 45bdf2c2df92..96dcace433cd 100644
--- a/external/xscom-utils/getscom.c
+++ b/external/xscom-utils/getscom.c
@@ -28,6 +28,7 @@  static void print_usage(void)
 	printf("usage: getscom [-c|--chip chip-id] addr\n");
 	printf("       getscom -l|--list-chips\n");
 	printf("       getscom -v|--version\n");
+	exit(1);
 }
 
 static void print_chip_info(uint32_t chip_id)
@@ -81,9 +82,7 @@  int main(int argc, char *argv[])
 {
 	uint64_t val, addr = -1ull;
 	uint32_t def_chip, chip_id = 0xffffffff;
-	bool show_help = false;
 	bool list_chips = false;
-	bool show_version = false;
 	bool no_work = false;
 	int rc;
 
@@ -107,14 +106,14 @@  int main(int argc, char *argv[])
 			chip_id = strtoul(optarg, NULL, 0);
 			break;
 		case 'h':
-			show_help = true;
+			print_usage();
 			break;
 		case 'l':
 			list_chips = true;
 			break;
 		case 'v':
-			show_version = true;
-			break;
+			printf("xscom utils version %s\n", VERSION_STR);
+			exit(1);
 		default:
 			exit(1);
 		}
@@ -122,17 +121,11 @@  int main(int argc, char *argv[])
 	
 	if (addr == -1ull)
 		no_work = true;
-	if (no_work && !list_chips && !show_version && !show_help) {
+	if (no_work && !list_chips) {
 		fprintf(stderr, "Invalid or missing address\n");
 		print_usage();
-		exit(1);
 	}
-	if (show_version)
-		printf("xscom utils version %s\n", VERSION_STR);
-	if (show_help)
-		print_usage();
-	if (no_work && !list_chips)
-		return 0;
+
 	def_chip = xscom_init();
 	if (def_chip == 0xffffffff) {
 		fprintf(stderr, "No valid XSCOM chip found\n");
diff --git a/external/xscom-utils/getsram.c b/external/xscom-utils/getsram.c
index 6e85fc88a0e0..ea3d38badf8d 100644
--- a/external/xscom-utils/getsram.c
+++ b/external/xscom-utils/getsram.c
@@ -29,6 +29,7 @@  static void print_usage(void)
 	printf("usage: getsram [-c|--chip chip-id] addr\n");
 	printf("               [--occ-channel|n <chan>]\n");
 	printf("       getsram -v|--version\n");
+	exit(1);
 }
 
 #define VERSION_STR _str(VERSION)
@@ -39,9 +40,6 @@  int main(int argc, char *argv[])
 {
 	uint64_t val, addr = -1ull;
 	uint32_t def_chip, chip_id = 0xffffffff;
-	bool show_help = false;
-	bool show_version = false;
-	bool no_work = false;
 	int rc;
 	int occ_channel = 0;
 
@@ -72,29 +70,21 @@  int main(int argc, char *argv[])
 			}
 			break;
 		case 'h':
-			show_help = true;
+			print_usage();
 			break;
 		case 'v':
-			show_version = true;
-			break;
+			printf("xscom utils version %s\n", VERSION_STR);
+			exit(1);
 		default:
 			exit(1);
 		}
 	}
 
-	if (addr == -1ull)
-		no_work = true;
-	if (no_work && !show_version && !show_help) {
+	if (addr == -1ull) {
 		fprintf(stderr, "Invalid or missing address\n");
 		print_usage();
-		exit(1);
 	}
-	if (show_version)
-		printf("xscom utils version %s\n", VERSION_STR);
-	if (show_help)
-		print_usage();
-	if (no_work)
-		return 0;
+
 	def_chip = xscom_init();
 	if (def_chip == 0xffffffff) {
 		fprintf(stderr, "No valid XSCOM chip found\n");
diff --git a/external/xscom-utils/putscom.c b/external/xscom-utils/putscom.c
index 8c397fc0be57..be7b696186cb 100644
--- a/external/xscom-utils/putscom.c
+++ b/external/xscom-utils/putscom.c
@@ -27,6 +27,7 @@  static void print_usage(void)
 {
 	printf("usage: putscom [-c|--chip chip-id] addr value\n");
 	printf("       putscom -v|--version\n");
+	exit(1);
 }
 
 #define VERSION_STR _str(VERSION)
@@ -37,9 +38,7 @@  int main(int argc, char *argv[])
 {
 	uint64_t val = -1ull, addr = -1ull;
 	uint32_t def_chip, chip_id = 0xffffffff;
-	bool show_help = false, got_addr = false, got_val = false;
-	bool show_version = false;
-	bool no_work = false;
+	bool got_addr = false, got_val = false;
 	int rc;
 
 	while(1) {
@@ -67,29 +66,21 @@  int main(int argc, char *argv[])
 			chip_id = strtoul(optarg, NULL, 0);
 			break;
 		case 'v':
-			show_version = true;
-			break;
+			printf("xscom utils version %s\n", VERSION_STR);
+			exit(1);
 		case 'h':
-			show_help = true;
+			print_usage();
 			break;
 		default:
 			exit(1);
 		}
 	}
 	
-	if (!got_addr || !got_val)
-		no_work = true;
-	if (no_work && !show_version && !show_help) {
+	if (!got_addr || !got_val) {
 		fprintf(stderr, "Invalid or missing address/value\n");
 		print_usage();
-		exit(1);
 	}
-	if (show_version)
-		printf("xscom utils version %s\n", VERSION_STR);
-	if (show_help)
-		print_usage();
-	if (no_work)
-		return 0;
+
 	def_chip = xscom_init();
 	if (def_chip == 0xffffffff) {
 		fprintf(stderr, "No valid XSCOM chip found\n");