Re: [PATCH] livepatch: fix confusing return value forenabled_store().

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 23/03/2015 21:44, Josh Poimboeuf wrote:
On Sun, Mar 22, 2015 at 10:15:07AM +0800, Xiubo Li wrote:
On 21/03/2015 23:52, Josh Poimboeuf wrote:
On Fri, Mar 20, 2015 at 04:58:46PM +0800, Xiubo Li wrote:
When enabling or disabling the live patch which is already in enabled
or disabled state, the /sys/kernel/livepatch/<patch>/enabled will just
return -EINVAL error value, so in user space will complaint like:
    "bash: echo: write error: Invalid argument"
and this will confuse users.
I actually prefer the current behavior.  I think the user should be
notified if they're trying to do something which doesn't make sense.

Yes, yes, I do agree with notifying the users that they're trying to do
something
which make no sense.

Days ago someone asked me about this live patching demo's error, and i
have to tell him why and showed him the source code, cause he isn't even
one programmer and isn't familiar with the Kernel code also, just think this
prompt is not very friendly and couldn't distinguish from the real Invalid
arguments and non sense operations.

If this change is not very acceptable, I'm okay with the current one.

Finally, just one suggestion, how about something like:
     pr_warning("This live patch has already been %s, do nothing!\n",
patch->state == KLP_ENABLE?"enabled":"disabled");
     return count;
Well, IMO, invalid user input isn't a good reason to pollute the printk
buffer.
   Maybe you could write a simple wrapper script which gives the
user a more helpful error message?

Maybe this is one acceptable choice !

Thanks,

BRs
Xiubo


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




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux