[Pdbg] [PATCH 05/10] pdbg: Tidy up putnia/putmsr

[rashmica]

On 15/06/18 15:12, Alistair Popple wrote:
> On Friday, 15 June 2018 1:50:55 PM AEST rashmica wrote:
>> On 15/06/18 11:26, Alistair Popple wrote:
>>> On Thursday, 31 May 2018 3:29:10 PM AEST Rashmica Gupta wrote:
>>>> The number of arguments required for these functions is wrong -
>>>> it requires 2 arguments for putnia and putmsr, and the second one
>>>> is written. The code has obviously been copy/pasted from getgpr/getspr
>>>> functions where you need to supply which register and then the data.
>>> The patch looks ok but I'm having trouble making sense of the commit message.
>>>
>>> Perhaps something like the below might be clearer?
>>>
>>> "putnia and putmsr only require 1 argument but the code currently checks for 2
>>> arguments and gets the data from the second argument which is wrong. It has
>>> obviously been copy/pasted from putgpr/putspr which do require 2 arguments. Fix
>>> this by reducing required arguments to 1 and getting the data from there."
>> That sounds a lot clearer! Do you need me to resend this?
> If you don't mind given there are a few other changes required for this series
> anyway.

I may have sent this patch separately as well as part of this series...
And you may have already merged the one by itself...

>
> - Alistair
>
>>> - Alistair
>>>
>>>> Signed-off-by: Rashmica Gupta <rashmica.g at gmail.com>
>>>> ---
>>>>  src/reg.c | 8 ++++----
>>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/src/reg.c b/src/reg.c
>>>> index 416236b..e252ab8 100644
>>>> --- a/src/reg.c
>>>> +++ b/src/reg.c
>>>> @@ -152,13 +152,13 @@ int handle_nia(int optind, int argc, char *argv[])
>>>>  	if (strcmp(argv[optind], "putnia") == 0) {
>>>>  		uint64_t data;
>>>>  
>>>> -		if (optind + 2 >= argc) {
>>>> +		if (optind + 1 >= argc) {
>>>>  			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
>>>>  			return -1;
>>>>  		}
>>>>  
>>>>  		errno = 0;
>>>> -		data = strtoull(argv[optind + 2], &endptr, 0);
>>>> +		data = strtoull(argv[optind + 1], &endptr, 0);
>>>>  		if (errno || *endptr != '\0') {
>>>>  			printf("%s: command '%s' couldn't parse data '%s'\n",
>>>>  				argv[0], argv[optind], argv[optind + 1]);
>>>> @@ -221,13 +221,13 @@ int handle_msr(int optind, int argc, char *argv[])
>>>>  	if (strcmp(argv[optind], "putmsr") == 0) {
>>>>  		uint64_t data;
>>>>  
>>>> -		if (optind + 2 >= argc) {
>>>> +		if (optind + 1 >= argc) {
>>>>  			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
>>>>  			return -1;
>>>>  		}
>>>>  
>>>>  		errno = 0;
>>>> -		data = strtoull(argv[optind + 2], &endptr, 0);
>>>> +		data = strtoull(argv[optind + 1], &endptr, 0);
>>>>  		if (errno || *endptr != '\0') {
>>>>  			printf("%s: command '%s' couldn't parse data '%s'\n",
>>>>  				argv[0], argv[optind], argv[optind + 1]);
>>>>
>>
>