Re: [PATCH 1/1] btrfs-progs: improve troubleshooting avoid duplicate error strings

[Anand Jain <Anand.Jain@oracle.com>]

On 04/14/2015 09:14 PM, David Sterba wrote:
> On Mon, Apr 13, 2015 at 08:37:01PM +0800, Anand Jain wrote:
>> my troubleshooting experience says have unique error string per module.
>>
>> In the below eg, its one additional step to know error line,
>>
>> cat -n cmds-device.c | egrep "error removing the device"
>>     185	"ERROR: error removing the device '%s' - %s\n",
>>     190	"ERROR: error removing the device '%s' - %s\n",
>>
>> which is completely avoidable.
>
> It is, we can merge both branches into one.



>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   cmds-device.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/cmds-device.c b/cmds-device.c
>> index 1c72e90..1c32771 100644
>> --- a/cmds-device.c
>> +++ b/cmds-device.c
>> @@ -187,7 +187,7 @@ static int cmd_rm_dev(int argc, char **argv)
>>   			ret++;
>>   		} else if (res < 0) {
>>   			fprintf(stderr,
>> -				"ERROR: error removing the device '%s' - %s\n",
>> +				"ERROR: ioctl error removing the device '%s' - %s\n",
>
> The only difference is the strerror vs btrfs_err_str. As both ret > 0
> and ret < 0 report some kind of error, the wording would be very similar
> so I think that one error message would fit better. I'll fix that.

You means res not ret (above) ?
--------------------------------------------------------------
diff --git a/cmds-device.c b/cmds-device.c
index cbb3243..1022656 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -183,7 +183,7 @@ static int cmd_rm_dev(int argc, char **argv)
                 if (res) {
                         const char *msg;

-                       if (ret > 0)
+                       if (res > 0)
                                 msg = btrfs_err_str(res);
                         else
                                 msg = strerror(e);
--------------------------------------------------------------

Thanks, Anand


> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>