Re: [Xen-devel] [PATCH v3 20/52] xen/arch/x86/shutdown.c: let custom parameter parsing routines return errno

[Jan Beulich]

>>> On 23.08.17 at 10:40,  wrote:
> On 22/08/17 11:53, Jan Beulich wrote:
> On 16.08.17 at 14:51,  wrote:
>>> @@ -82,6 +87,8 @@ static void __init set_reboot_type(char *str)
>>>  
>>>  if ( reboot_type == BOOT_EFI && !efi_enabled(EFI_RS) )
>>>  reboot_type = BOOT_INVALID;
>> 
>> Should this perhaps also lead to -EINVAL being returned?
> 
> Hmm, I'm not sure. The parameter as such was valid.
> 
> So maybe a message right here would be the better solution?

I'd be fine with that too, it's just that with the overall change
your series does this shouldn't go silent anymore.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3 20/52] xen/arch/x86/shutdown.c: let custom parameter parsing routines return errno

[Juergen Gross]

On 22/08/17 11:53, Jan Beulich wrote:
 On 16.08.17 at 14:51,  wrote:
>> --- a/xen/arch/x86/shutdown.c
>> +++ b/xen/arch/x86/shutdown.c
>> @@ -51,8 +51,11 @@ static int reboot_mode;
>>   * efiUse the EFI reboot (if running under EFI)
>>   */
>>  static enum reboot_type reboot_type = BOOT_INVALID;
>> -static void __init set_reboot_type(char *str)
>> +
>> +static int __init set_reboot_type(const char *str)
>>  {
>> +int rc = 0;
>> +
>>  for ( ; ; )
>>  {
>>  switch ( *str )
>> @@ -74,6 +77,8 @@ static void __init set_reboot_type(char *str)
>>  case 't':
>>  reboot_type = *str;
>>  break;
>> +default:
>> +rc = -EINVAL;
>>  }
> 
> Please don't omit the break statement, even if it is not strictly needed
> here.

Okay.

> 
>> @@ -82,6 +87,8 @@ static void __init set_reboot_type(char *str)
>>  
>>  if ( reboot_type == BOOT_EFI && !efi_enabled(EFI_RS) )
>>  reboot_type = BOOT_INVALID;
> 
> Should this perhaps also lead to -EINVAL being returned?

Hmm, I'm not sure. The parameter as such was valid.

So maybe a message right here would be the better solution?


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3 20/52] xen/arch/x86/shutdown.c: let custom parameter parsing routines return errno

[Jan Beulich]

>>> On 16.08.17 at 14:51,  wrote:
> --- a/xen/arch/x86/shutdown.c
> +++ b/xen/arch/x86/shutdown.c
> @@ -51,8 +51,11 @@ static int reboot_mode;
>   * efiUse the EFI reboot (if running under EFI)
>   */
>  static enum reboot_type reboot_type = BOOT_INVALID;
> -static void __init set_reboot_type(char *str)
> +
> +static int __init set_reboot_type(const char *str)
>  {
> +int rc = 0;
> +
>  for ( ; ; )
>  {
>  switch ( *str )
> @@ -74,6 +77,8 @@ static void __init set_reboot_type(char *str)
>  case 't':
>  reboot_type = *str;
>  break;
> +default:
> +rc = -EINVAL;
>  }

Please don't omit the break statement, even if it is not strictly needed
here.

> @@ -82,6 +87,8 @@ static void __init set_reboot_type(char *str)
>  
>  if ( reboot_type == BOOT_EFI && !efi_enabled(EFI_RS) )
>  reboot_type = BOOT_INVALID;

Should this perhaps also lead to -EINVAL being returned?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v3 20/52] xen/arch/x86/shutdown.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/shutdown.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
---
V3:
- dont stop loop at first invalid character (Jan Beulich)
---
 xen/arch/x86/shutdown.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c
index f63b8a668f..862384a8eb 100644
--- a/xen/arch/x86/shutdown.c
+++ b/xen/arch/x86/shutdown.c
@@ -51,8 +51,11 @@ static int reboot_mode;
  * efiUse the EFI reboot (if running under EFI)
  */
 static enum reboot_type reboot_type = BOOT_INVALID;
-static void __init set_reboot_type(char *str)
+
+static int __init set_reboot_type(const char *str)
 {
+int rc = 0;
+
 for ( ; ; )
 {
 switch ( *str )
@@ -74,6 +77,8 @@ static void __init set_reboot_type(char *str)
 case 't':
 reboot_type = *str;
 break;
+default:
+rc = -EINVAL;
 }
 if ( (str = strchr(str, ',')) == NULL )
 break;
@@ -82,6 +87,8 @@ static void __init set_reboot_type(char *str)
 
 if ( reboot_type == BOOT_EFI && !efi_enabled(EFI_RS) )
 reboot_type = BOOT_INVALID;
+
+return rc;
 }
 custom_param("reboot", set_reboot_type);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel