Re: [qemu-s390x] [PATCH v2] pc-bios/s390-ccw: Move string arrays from bootmap header to .c file

[Christian Borntraeger]

On 03/06/2018 07:18 AM, Thomas Huth wrote:
> bootmap.h can currently only be included once - otherwise the linker
> complains about multiple definitions of the "magic" strings. It's a
> bad style to define string arrays in header files, so let's better
> move these to the bootmap.c file instead where they are used.
> 
> Signed-off-by: Thomas Huth <address@hidden>

Reviewed-by: Christian Borntraeger <address@hidden>

> ---
>  v2:
>  - Removed duplicated vol_desc_magic (copy-n-paste error)
> 
>  pc-bios/s390-ccw/bootmap.c | 20 ++++++++++++++++++++
>  pc-bios/s390-ccw/bootmap.h | 19 -------------------
>  2 files changed, 20 insertions(+), 19 deletions(-)
> 
> diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
> index 29bfd8c..fc2a9fe 100644
> --- a/pc-bios/s390-ccw/bootmap.c
> +++ b/pc-bios/s390-ccw/bootmap.c
> @@ -37,6 +37,26 @@ typedef struct ResetInfo {
> 
>  static ResetInfo save;
> 
> +const uint8_t el_torito_magic[] = "EL TORITO SPECIFICATION"
> +                                  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0";
> +
> +/*
> + * Match two CCWs located after PSW and eight filler bytes.
> + * From libmagic and arch/s390/kernel/head.S.
> + */
> +const uint8_t linux_s390_magic[] = "\x02\x00\x00\x18\x60\x00\x00\x50\x02\x00"
> +                                   "\x00\x68\x60\x00\x00\x50\x40\x40\x40\x40"
> +                                   "\x40\x40\x40\x40";
> +
> +static inline bool is_iso_vd_valid(IsoVolDesc *vd)
> +{
> +    const uint8_t vol_desc_magic[] = "CD001";
> +
> +    return !memcmp(&vd->ident[0], vol_desc_magic, 5) &&
> +           vd->version == 0x1 &&
> +           vd->type <= VOL_DESC_TYPE_PARTITION;
> +}
> +
>  static void jump_to_IPL_2(void)
>  {
>      ResetInfo *current = 0;
> diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
> index c636626..07eb600 100644
> --- a/pc-bios/s390-ccw/bootmap.h
> +++ b/pc-bios/s390-ccw/bootmap.h
> @@ -375,9 +375,6 @@ static inline void read_iso_boot_image(uint32_t 
> block_offset, void *load_addr,
>                 "Failed to read boot image!");
>  }
> 
> -const uint8_t el_torito_magic[] = "EL TORITO SPECIFICATION"
> -                                  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0";
> -
>  #define ISO9660_MAX_DIR_DEPTH 8
> 
>  typedef struct IsoDirHdr {
> @@ -430,20 +427,12 @@ typedef struct IsoVolDesc {
>      } vd;
>  } __attribute__((packed)) IsoVolDesc;
> 
> -const uint8_t vol_desc_magic[] = "CD001";
>  #define VOL_DESC_TYPE_BOOT 0
>  #define VOL_DESC_TYPE_PRIMARY 1
>  #define VOL_DESC_TYPE_SUPPLEMENT 2
>  #define VOL_DESC_TYPE_PARTITION 3
>  #define VOL_DESC_TERMINATOR 255
> 
> -static inline bool is_iso_vd_valid(IsoVolDesc *vd)
> -{
> -    return !memcmp(&vd->ident[0], vol_desc_magic, 5) &&
> -           vd->version == 0x1 &&
> -           vd->type <= VOL_DESC_TYPE_PARTITION;
> -}
> -
>  typedef struct IsoBcValid {
>      uint8_t platform_id;
>      uint16_t reserved;
> @@ -468,14 +457,6 @@ typedef struct IsoBcHdr {
>      uint8_t id[28];
>  } __attribute__((packed)) IsoBcHdr;
> 
> -/*
> - * Match two CCWs located after PSW and eight filler bytes.
> - * From libmagic and arch/s390/kernel/head.S.
> - */
> -const uint8_t linux_s390_magic[] = "\x02\x00\x00\x18\x60\x00\x00\x50\x02\x00"
> -                                   "\x00\x68\x60\x00\x00\x50\x40\x40\x40\x40"
> -                                   "\x40\x40\x40\x40";
> -
>  typedef struct IsoBcEntry {
>      uint8_t id;
>      union {
>