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

[Eric Blake]

On 03/06/2018 12:18 AM, Thomas Huth wrote:
> bootmap.h can currently only be included once - otherwise the linker
> complains about multiple definitions of the "magic" strings.

My first thought when reading that was "Huh? bootmap.h has a proper[*] 
double-inclusion header guard, and therefore a second #include 
"bootmap.h" is a no-op - so how can including the header more than once 
cause a linker complaint?"

[*] Well, proper if you overlook the fact that the name 
_PC_BIOS_S390_CCW_BOOTMAP_H starts with a leading underscore followed by 
uppercase, and is therefore violating namespace safety rules, as it 
could collide with a symbol reserved for the implementation

> 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.

But I finally figured out what you really meant: if more than one .c 
file each include the header (and not my initial reading of a single .c 
file including the header more than once), then since the header was 
declaring non-static top-level variables, that does indeed cause linker 
errors.

> Signed-off-by: Thomas Huth <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(-)

Your change is fine (moving the declaration into the one .c file that 
needs them), so no need to change this, but...

> +++ 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";

...would adding 'static' here also solved the linker error (at the risk 
of possibly causing a compiler warning/error about unused variable)?

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org