diff mbox

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

Message ID 554C73A8.1030406@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Anand Jain May 8, 2015, 8:28 a.m. UTC 
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) ?
--------------------------------------------------------------
--------------------------------------------------------------

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
>
--
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

Comments

David Sterba May 11, 2015, 11:23 a.m. UTC | #1 
On Fri, May 08, 2015 at 04:28:24PM +0800, Anand Jain wrote:
> >> @@ -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) ?

Oh right, please send a patch.
--
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
diff mbox

Patch

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);