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