reg.c: Refactor get/put register handling

We already parse the difference between get and put register so refactor to
avoid parsing a second time.

Signed-off-by: Alistair Popple <alistair@popple.id.au>
---
 src/main.c |  16 +++---
 src/reg.c  | 166 +++++++++++++++++++++++++++++++++----------------------------
 src/reg.h  |  12 +++--
 3 files changed, 106 insertions(+), 88 deletions(-)

NAKing my own patch :-)

This has the same bug Rashmica fixed for putmsr and putnia (expecting two
arguments when only one is required).

I am currently looking at alternate ways of doing command parsing/processing
anyway so I will leave this for a future cleanup for now.

- Alistair

On Friday, 25 May 2018 2:26:35 PM AEST Alistair Popple wrote:
> We already parse the difference between get and put register so refactor to
> avoid parsing a second time.
> 
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
> ---
>  src/main.c |  16 +++---
>  src/reg.c  | 166 +++++++++++++++++++++++++++++++++----------------------------
>  src/reg.h  |  12 +++--
>  3 files changed, 106 insertions(+), 88 deletions(-)
> 
> diff --git a/src/main.c b/src/main.c
> index 85770ef..5bc3222 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -88,14 +88,14 @@ struct action {
>  };
>  
>  static struct action actions[] = {
> -	{ "getgpr",  "<gpr>", "Read General Purpose Register (GPR)", &handle_gpr },
> -	{ "putgpr",  "<gpr> <value>", "Write General Purpose Register (GPR)", &handle_gpr },
> -	{ "getnia",  "", "Get Next Instruction Address (NIA)", &handle_nia },
> -	{ "putnia",  "<value>", "Write Next Instrution Address (NIA)", &handle_nia },
> -	{ "getspr",  "<spr>", "Get Special Purpose Register (SPR)", &handle_spr },
> -	{ "putspr",  "<spr> <value>", "Write Special Purpose Register (SPR)", &handle_spr },
> -	{ "getmsr",  "", "Get Machine State Register (MSR)", &handle_msr },
> -	{ "putmsr",  "<value>", "Write Machine State Register (MSR)", &handle_msr },
> +	{ "getgpr",  "<gpr>", "Read General Purpose Register (GPR)", &handle_getgpr },
> +	{ "putgpr",  "<gpr> <value>", "Write General Purpose Register (GPR)", &handle_putgpr },
> +	{ "getnia",  "", "Get Next Instruction Address (NIA)", &handle_getnia },
> +	{ "putnia",  "<value>", "Write Next Instrution Address (NIA)", &handle_putnia },
> +	{ "getspr",  "<spr>", "Get Special Purpose Register (SPR)", &handle_getspr },
> +	{ "putspr",  "<spr> <value>", "Write Special Purpose Register (SPR)", &handle_putspr },
> +	{ "getmsr",  "", "Get Machine State Register (MSR)", &handle_getmsr },
> +	{ "putmsr",  "<value>", "Write Machine State Register (MSR)", &handle_putmsr },
>  	{ "getring", "<addr> <len>", "Read a ring. Length must be correct", &handle_getring },
>  	{ "start",   "", "Start thread", &thread_start },
>  	{ "step",    "<count>", "Set a thread <count> instructions", &thread_step },
> diff --git a/src/reg.c b/src/reg.c
> index 002cfe9..932ae43 100644
> --- a/src/reg.c
> +++ b/src/reg.c
> @@ -91,10 +91,10 @@ static int getprocreg(struct pdbg_target *target, uint32_t index, uint64_t *reg,
>  	return !rc;
>  }
>  
> -int handle_gpr(int optind, int argc, char *argv[])
> +static int parse_gpr(int optind, int argc, char *argv[])
>  {
>  	char *endptr;
> -	uint64_t gpr;
> +	int gpr;
>  
>  	if (optind + 1 >= argc) {
>  		printf("%s: command '%s' requires a GPR\n", argv[0], argv[optind]);
> @@ -102,10 +102,10 @@ int handle_gpr(int optind, int argc, char *argv[])
>  	}
>  
>  	errno = 0;
> -	gpr = strtoull(argv[optind + 1], &endptr, 0);
> +	gpr = strtoul(argv[optind + 1], &endptr, 0);
>  	if (errno || *endptr != '\0') {
>  		printf("%s: command '%s' couldn't parse GPR '%s'\n",
> -				argv[0], argv[optind], argv[optind + 1]);
> +		       argv[0], argv[optind], argv[optind + 1]);
>  		return -1;
>  	}
>  
> @@ -114,120 +114,134 @@ int handle_gpr(int optind, int argc, char *argv[])
>  		return -1;
>  	}
>  
> -	if (strcmp(argv[optind], "putgpr") == 0) {
> -		uint64_t data;
> +	return gpr;
> +}
>  
> -		if (optind + 2 >= argc) {
> -			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
> -			return -1;
> -		}
> +static int parse_data(int optind, int argc, char *argv[], uint64_t *data)
> +{
> +	char *endptr;
>  
> -		errno = 0;
> -		data = strtoull(argv[optind + 2], &endptr, 0);
> -		if (errno || *endptr != '\0') {
> -			printf("%s: command '%s' couldn't parse data '%s'\n",
> -				argv[0], argv[optind], argv[optind + 1]);
> -			return -1;
> -		}
> +	if (optind + 2 >= argc) {
> +		printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
> +		return -1;
> +	}
>  
> -		return for_each_target("thread", putprocreg, &gpr, &data);
> +	errno = 0;
> +	*data = strtoull(argv[optind + 2], &endptr, 0);
> +	if (errno || *endptr != '\0') {
> +		printf("%s: command '%s' couldn't parse data '%s'\n",
> +		       argv[0], argv[optind], argv[optind + 1]);
> +		return -1;
>  	}
>  
> +	return 0;
> +}
> +
> +int handle_getgpr(int optind, int argc, char *argv[])
> +{
> +	uint64_t gpr;
> +
> +	if ((gpr = parse_gpr(optind, argc, argv)) < 0)
> +		return -1;
> +
>  	return for_each_target("thread", getprocreg, &gpr, NULL);
>  }
>  
> -int handle_nia(int optind, int argc, char *argv[])
> +int handle_putgpr(int optind, int argc, char *argv[])
>  {
> -	uint64_t reg = REG_NIA;
> -	char *endptr;
> +	uint64_t gpr, data;
>  
> -	if (strcmp(argv[optind], "putnia") == 0) {
> -		uint64_t data;
> +	if ((gpr = parse_gpr(optind, argc, argv)) < 0)
> +		return -1;
>  
> -		if (optind + 2 >= argc) {
> -			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
> -			return -1;
> -		}
> +	if (parse_data(optind, argc, argv, &data))
> +		return -1;
>  
> -		errno = 0;
> -		data = strtoull(argv[optind + 2], &endptr, 0);
> -		if (errno || *endptr != '\0') {
> -			printf("%s: command '%s' couldn't parse data '%s'\n",
> -				argv[0], argv[optind], argv[optind + 1]);
> -			return -1;
> -		}
> +	return for_each_target("thread", putprocreg, &gpr, &data);
> +}
>  
> -		return for_each_target("thread", putprocreg, &reg, &data);
> -	}
> +int handle_getnia(int optind, int argc, char *argv[])
> +{
> +	uint64_t reg = REG_NIA;
>  
>  	return for_each_target("thread", getprocreg, &reg, NULL);
>  }
>  
> -int handle_spr(int optind, int argc, char *argv[])
> +int handle_putnia(int optind, int argc, char *argv[])
> +{
> +	uint64_t reg = REG_NIA, data;
> +
> +	if (parse_data(optind, argc, argv, &data))
> +		return -1;
> +
> +	return for_each_target("thread", putprocreg, &reg, &data);
> +}
> +
> +static int parse_spr(int optind, int argc, char *argv[])
>  {
>  	char *endptr;
> -	uint64_t spr;
> +	int gpr;
>  
>  	if (optind + 1 >= argc) {
> -		printf("%s: command '%s' requires a GPR\n", argv[0], argv[optind]);
> +		printf("%s: command '%s' requires a SPR\n", argv[0], argv[optind]);
>  		return -1;
>  	}
>  
>  	errno = 0;
> -	spr = strtoull(argv[optind + 1], &endptr, 0);
> +	gpr = strtoul(argv[optind + 1], &endptr, 0);
>  	if (errno || *endptr != '\0') {
> -		printf("%s: command '%s' couldn't parse GPR '%s'\n",
> -				argv[0], argv[optind], argv[optind + 1]);
> +		printf("%s: command '%s' couldn't parse SPR '%s'\n",
> +		       argv[0], argv[optind], argv[optind + 1]);
>  		return -1;
>  	}
>  
> -	spr += REG_R31;
> +	if (gpr > 0x3ff) {
> +		printf("A SPR must be between zero and 1023 inclusive\n");
> +		return -1;
> +	}
>  
> -	if (strcmp(argv[optind], "putspr") == 0) {
> -		uint64_t data;
> +	return gpr;
> +}
>  
> -		if (optind + 2 >= argc) {
> -			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
> -			return -1;
> -		}
> +int handle_getspr(int optind, int argc, char *argv[])
> +{
> +	uint64_t spr;
>  
> -		errno = 0;
> -		data = strtoull(argv[optind + 2], &endptr, 0);
> -		if (errno || *endptr != '\0') {
> -			printf("%s: command '%s' couldn't parse data '%s'\n",
> -				argv[0], argv[optind], argv[optind + 1]);
> -			return -1;
> -		}
> +	if ((spr = parse_spr(optind, argc, argv)) < 0)
> +		return -1;
>  
> -		return for_each_target("thread", putprocreg, &spr, &data);
> -	}
> +	spr += REG_R31;
>  
>  	return for_each_target("thread", getprocreg, &spr, NULL);
>  }
>  
> -int handle_msr(int optind, int argc, char *argv[])
> +int handle_putspr(int optind, int argc, char *argv[])
>  {
> -	uint64_t msr = REG_MSR;
> -	char *endptr;
> +	uint64_t spr, data;
>  
> -	if (strcmp(argv[optind], "putmsr") == 0) {
> -		uint64_t data;
> +	if ((spr = parse_spr(optind, argc, argv)) < 0)
> +		return -1;
>  
> -		if (optind + 2 >= argc) {
> -			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
> -			return -1;
> -		}
> +	spr += REG_R31;
> +	if (parse_data(optind, argc, argv, &data))
> +		return -1;
>  
> -		errno = 0;
> -		data = strtoull(argv[optind + 2], &endptr, 0);
> -		if (errno || *endptr != '\0') {
> -			printf("%s: command '%s' couldn't parse data '%s'\n",
> -				argv[0], argv[optind], argv[optind + 1]);
> -			return -1;
> -		}
> +	return for_each_target("thread", putprocreg, &spr, &data);
> +}
>  
> -		return for_each_target("thread", putprocreg, &msr, &data);
> -	}
> +int handle_getmsr(int optind, int argc, char *argv[])
> +{
> +	uint64_t msr = REG_MSR;
>  
>  	return for_each_target("thread", getprocreg, &msr, NULL);
>  }
> +
> +int handle_putmsr(int optind, int argc, char *argv[])
> +{
> +	uint64_t msr = REG_MSR, data;
> +
> +	if (parse_data(optind, argc, argv, &data))
> +		return -1;
> +
> +	return for_each_target("thread", putprocreg, &msr, &data);
> +}
> diff --git a/src/reg.h b/src/reg.h
> index ad41d9d..9ee2cd4 100644
> --- a/src/reg.h
> +++ b/src/reg.h
> @@ -14,7 +14,11 @@
>   * limitations under the License.
>   */
>  
> -int handle_gpr(int optind, int argc, char *argv[]);
> -int handle_nia(int optind, int argc, char *argv[]);
> -int handle_spr(int optind, int argc, char *argv[]);
> -int handle_msr(int optind, int argc, char *argv[]);
> +int handle_getgpr(int optind, int argc, char *argv[]);
> +int handle_putgpr(int optind, int argc, char *argv[]);
> +int handle_getnia(int optind, int argc, char *argv[]);
> +int handle_putnia(int optind, int argc, char *argv[]);
> +int handle_getspr(int optind, int argc, char *argv[]);
> +int handle_putspr(int optind, int argc, char *argv[]);
> +int handle_getmsr(int optind, int argc, char *argv[]);
> +int handle_putmsr(int optind, int argc, char *argv[]);
>