Re: [Xen-devel] [PATCH RFC 6/6] xen/keyhandler: Drop keyhandler_scratch

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.09.18 at 14:08, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -489,7 +489,7 @@ static EFI_FILE_HANDLE __init 
> get_parent_handle(EFI_LOADED_IMAGE *loaded_image,
>      static EFI_GUID __initdata fs_protocol = SIMPLE_FILE_SYSTEM_PROTOCOL;
>      EFI_FILE_HANDLE dir_handle;
>      EFI_DEVICE_PATH *dp;
> -    CHAR16 *pathend, *ptr;
> +    CHAR16 *pathend, *ptr, buffer[128];
>      EFI_STATUS ret;
>  
>      do {
> @@ -506,8 +506,7 @@ static EFI_FILE_HANDLE __init 
> get_parent_handle(EFI_LOADED_IMAGE *loaded_image,
>      if ( ret != EFI_SUCCESS )
>          PrintErrMesg(L"OpenVolume failure", ret);
>  
> -#define buffer ((CHAR16 *)keyhandler_scratch)
> -#define BUFFERSIZE sizeof(keyhandler_scratch)
> +#define BUFFERSIZE sizeof(buffer)
>      for ( dp = loaded_image->FilePath, *buffer = 0;
>            DevicePathType(dp) != END_DEVICE_PATH_TYPE;
>            dp = (void *)dp + DevicePathNodeLength(dp) )

I don't mind a change like this at all - it just was convenient to use
an available static buffer. I'm not overly happy about the large stack
item - did you consider using a static __initdata object instead? We
could then also consider growing it further, as I'm not sure 128 is
sufficient in the general case (but then again I'm also not sure
whether FAT32 has an upper bound on file name length - it's been
too long ago that I last had to play with related code).

> @@ -333,8 +317,13 @@ static void dump_domains(unsigned char key)
>              printk("    pause_count=%d pause_flags=%lx\n",
>                     atomic_read(&v->pause_count), v->pause_flags);
>              arch_dump_vcpu_info(v);
> -            periodic_timer_print(tmpstr, sizeof(tmpstr), v->periodic_period);
> -            printk("    %s\n", tmpstr);
> +
> +            if ( v->periodic_period == 0 )
> +                printk("No periodic timer\n");
> +            else
> +                printk("%u Hz periodic timer (period %u ms)\n",
> +                       1000000000/(int)v->periodic_period,
> +                       (int)v->periodic_period/1000000);

Blanks around / please, and I can't see why the casts would be
needed either (in fact ).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel