[Pdbg] [PATCH] reg.c: Refactor get/put register handling
Alistair Popple
alistair at popple.id.au
Fri Jun 1 13:23:43 AEST 2018
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 at 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, ®, &data);
> - }
> +int handle_getnia(int optind, int argc, char *argv[])
> +{
> + uint64_t reg = REG_NIA;
>
> return for_each_target("thread", getprocreg, ®, 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, ®, &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[]);
>
More information about the Pdbg
mailing list