diff mbox series

x86/boot: Use C99 types for integers

Message ID 20240829115247.168608-1-frediano.ziglio@cloud.com (mailing list archive)
State New
Headers show
Series x86/boot: Use C99 types for integers | expand

Commit Message

Frediano Ziglio Aug. 29, 2024, 11:52 a.m. UTC 
Just style update, no functional change.

Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
---
 xen/arch/x86/boot/cmdline.c | 30 ++++++++++++++---------------
 xen/arch/x86/boot/defs.h    |  2 +-
 xen/arch/x86/boot/reloc.c   | 38 ++++++++++++++++++-------------------
 3 files changed, 35 insertions(+), 35 deletions(-)

Comments

Andrew Cooper Aug. 29, 2024, 12:07 p.m. UTC | #1 
On 29/08/2024 12:52 pm, Frediano Ziglio wrote:
> diff --git a/xen/arch/x86/boot/defs.h b/xen/arch/x86/boot/defs.h
> index 239b9f8716..ee1a4da6af 100644
> --- a/xen/arch/x86/boot/defs.h
> +++ b/xen/arch/x86/boot/defs.h
> @@ -57,7 +57,7 @@ typedef u16 uint16_t;
>  typedef u32 uint32_t;
>  typedef u64 uint64_t;
>  
> -#define U16_MAX		((u16)(~0U))
> +#define UINT16_MAX	((uint16_t)(~0U))
>  #define UINT_MAX	(~0U)
>  
>  #endif /* __BOOT_DEFS_H__ */

I'm happy with the change in principle, but could we see about dropping
defs.h entirely?  For example, we've already got both of these UINT
constants in types.h

Since this was written, we've got rather better about cleaning up
xen/types.h, and extracting macros into xen/macros.h

I think there's a good chance that the regular headers can now be used
directly, or with minor tweaking.

~Andrew
Frediano Ziglio Aug. 29, 2024, 1:42 p.m. UTC | #2 
On Thu, Aug 29, 2024 at 1:07 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 29/08/2024 12:52 pm, Frediano Ziglio wrote:
> > diff --git a/xen/arch/x86/boot/defs.h b/xen/arch/x86/boot/defs.h
> > index 239b9f8716..ee1a4da6af 100644
> > --- a/xen/arch/x86/boot/defs.h
> > +++ b/xen/arch/x86/boot/defs.h
> > @@ -57,7 +57,7 @@ typedef u16 uint16_t;
> >  typedef u32 uint32_t;
> >  typedef u64 uint64_t;
> >
> > -#define U16_MAX              ((u16)(~0U))
> > +#define UINT16_MAX   ((uint16_t)(~0U))
> >  #define UINT_MAX     (~0U)
> >
> >  #endif /* __BOOT_DEFS_H__ */
>
> I'm happy with the change in principle, but could we see about dropping
> defs.h entirely?  For example, we've already got both of these UINT
> constants in types.h
>
> Since this was written, we've got rather better about cleaning up
> xen/types.h, and extracting macros into xen/macros.h
>
> I think there's a good chance that the regular headers can now be used
> directly, or with minor tweaking.
>

I tried, it gave a huge bunch of errors.
I think it can be done in a later follow-up.

Frediano
Andrew Cooper Sept. 2, 2024, 10:16 a.m. UTC | #3 
On 29/08/2024 2:42 pm, Frediano Ziglio wrote:
> On Thu, Aug 29, 2024 at 1:07 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 29/08/2024 12:52 pm, Frediano Ziglio wrote:
>>> diff --git a/xen/arch/x86/boot/defs.h b/xen/arch/x86/boot/defs.h
>>> index 239b9f8716..ee1a4da6af 100644
>>> --- a/xen/arch/x86/boot/defs.h
>>> +++ b/xen/arch/x86/boot/defs.h
>>> @@ -57,7 +57,7 @@ typedef u16 uint16_t;
>>>  typedef u32 uint32_t;
>>>  typedef u64 uint64_t;
>>>
>>> -#define U16_MAX              ((u16)(~0U))
>>> +#define UINT16_MAX   ((uint16_t)(~0U))
>>>  #define UINT_MAX     (~0U)
>>>
>>>  #endif /* __BOOT_DEFS_H__ */
>> I'm happy with the change in principle, but could we see about dropping
>> defs.h entirely?  For example, we've already got both of these UINT
>> constants in types.h
>>
>> Since this was written, we've got rather better about cleaning up
>> xen/types.h, and extracting macros into xen/macros.h
>>
>> I think there's a good chance that the regular headers can now be used
>> directly, or with minor tweaking.
>>
> I tried, it gave a huge bunch of errors.
> I think it can be done in a later follow-up.

Ok fine.

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

However, doesn't this mean we can drop the block of typedefs in the
context above?  Happy to fold that in on commit.

~Andrew
Andrew Cooper Sept. 2, 2024, 10:42 a.m. UTC | #4 
On 02/09/2024 11:16 am, Andrew Cooper wrote:
> On 29/08/2024 2:42 pm, Frediano Ziglio wrote:
>> On Thu, Aug 29, 2024 at 1:07 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>> On 29/08/2024 12:52 pm, Frediano Ziglio wrote:
>>>> diff --git a/xen/arch/x86/boot/defs.h b/xen/arch/x86/boot/defs.h
>>>> index 239b9f8716..ee1a4da6af 100644
>>>> --- a/xen/arch/x86/boot/defs.h
>>>> +++ b/xen/arch/x86/boot/defs.h
>>>> @@ -57,7 +57,7 @@ typedef u16 uint16_t;
>>>>  typedef u32 uint32_t;
>>>>  typedef u64 uint64_t;
>>>>
>>>> -#define U16_MAX              ((u16)(~0U))
>>>> +#define UINT16_MAX   ((uint16_t)(~0U))
>>>>  #define UINT_MAX     (~0U)
>>>>
>>>>  #endif /* __BOOT_DEFS_H__ */
>>> I'm happy with the change in principle, but could we see about dropping
>>> defs.h entirely?  For example, we've already got both of these UINT
>>> constants in types.h
>>>
>>> Since this was written, we've got rather better about cleaning up
>>> xen/types.h, and extracting macros into xen/macros.h
>>>
>>> I think there's a good chance that the regular headers can now be used
>>> directly, or with minor tweaking.
>>>
>> I tried, it gave a huge bunch of errors.
>> I think it can be done in a later follow-up.
> Ok fine.
>
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> However, doesn't this mean we can drop the block of typedefs in the
> context above?  Happy to fold that in on commit.

Nope, that breaks differently.

I'll put the patch in as-is and cleanup can come later.

~Andrew
diff mbox series

Patch

diff --git a/xen/arch/x86/boot/cmdline.c b/xen/arch/x86/boot/cmdline.c
index f9eee756aa..b8ad7f3a14 100644
--- a/xen/arch/x86/boot/cmdline.c
+++ b/xen/arch/x86/boot/cmdline.c
@@ -36,15 +36,15 @@  asm (
 
 /* Keep in sync with trampoline.S:early_boot_opts label! */
 typedef struct __packed {
-    u8 skip_realmode;
-    u8 opt_edd;
-    u8 opt_edid;
-    u8 padding;
+    uint8_t skip_realmode;
+    uint8_t opt_edd;
+    uint8_t opt_edid;
+    uint8_t padding;
 #ifdef CONFIG_VIDEO
-    u16 boot_vid_mode;
-    u16 vesa_width;
-    u16 vesa_height;
-    u16 vesa_depth;
+    uint16_t boot_vid_mode;
+    uint16_t vesa_width;
+    uint16_t vesa_height;
+    uint16_t vesa_depth;
 #endif
 } early_boot_opts_t;
 
@@ -214,7 +214,7 @@  static bool skip_realmode(const char *cmdline)
     return find_opt(cmdline, "no-real-mode", false) || find_opt(cmdline, "tboot=", true);
 }
 
-static u8 edd_parse(const char *cmdline)
+static uint8_t edd_parse(const char *cmdline)
 {
     const char *c;
 
@@ -229,7 +229,7 @@  static u8 edd_parse(const char *cmdline)
     return !strmaxcmp(c, "skipmbr", delim_chars);
 }
 
-static u8 edid_parse(const char *cmdline)
+static uint8_t edid_parse(const char *cmdline)
 {
     const char *c;
 
@@ -245,7 +245,7 @@  static u8 edid_parse(const char *cmdline)
 }
 
 #ifdef CONFIG_VIDEO
-static u16 rows2vmode(unsigned int rows)
+static uint16_t rows2vmode(unsigned int rows)
 {
     switch ( rows )
     {
@@ -300,7 +300,7 @@  static void vga_parse(const char *cmdline, early_boot_opts_t *ebo)
         {
             vesa_width = strtoui(c + strlen("gfx-"), "x", &c);
 
-            if ( vesa_width > U16_MAX )
+            if ( vesa_width > UINT16_MAX )
                 return;
 
             /*
@@ -311,12 +311,12 @@  static void vga_parse(const char *cmdline, early_boot_opts_t *ebo)
             ++c;
             vesa_height = strtoui(c, "x", &c);
 
-            if ( vesa_height > U16_MAX )
+            if ( vesa_height > UINT16_MAX )
                 return;
 
             vesa_depth = strtoui(++c, delim_chars_comma, NULL);
 
-            if ( vesa_depth > U16_MAX )
+            if ( vesa_depth > UINT16_MAX )
                 return;
 
             ebo->vesa_width = vesa_width;
@@ -328,7 +328,7 @@  static void vga_parse(const char *cmdline, early_boot_opts_t *ebo)
         {
             tmp = strtoui(c + strlen("mode-"), delim_chars_comma, NULL);
 
-            if ( tmp > U16_MAX )
+            if ( tmp > UINT16_MAX )
                 return;
 
             ebo->boot_vid_mode = tmp;
diff --git a/xen/arch/x86/boot/defs.h b/xen/arch/x86/boot/defs.h
index 239b9f8716..ee1a4da6af 100644
--- a/xen/arch/x86/boot/defs.h
+++ b/xen/arch/x86/boot/defs.h
@@ -57,7 +57,7 @@  typedef u16 uint16_t;
 typedef u32 uint32_t;
 typedef u64 uint64_t;
 
-#define U16_MAX		((u16)(~0U))
+#define UINT16_MAX	((uint16_t)(~0U))
 #define UINT_MAX	(~0U)
 
 #endif /* __BOOT_DEFS_H__ */
diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c
index 4033557481..589e026ff9 100644
--- a/xen/arch/x86/boot/reloc.c
+++ b/xen/arch/x86/boot/reloc.c
@@ -68,24 +68,24 @@  struct vesa_mode_info {
 #endif /* CONFIG_VIDEO */
 
 #define get_mb2_data(tag, type, member)   (((const multiboot2_tag_##type##_t *)(tag))->member)
-#define get_mb2_string(tag, type, member) ((u32)get_mb2_data(tag, type, member))
+#define get_mb2_string(tag, type, member) ((uint32_t)get_mb2_data(tag, type, member))
 
-static u32 alloc;
+static uint32_t alloc;
 
-static u32 alloc_mem(u32 bytes)
+static uint32_t alloc_mem(uint32_t bytes)
 {
     return alloc -= ALIGN_UP(bytes, 16);
 }
 
-static void zero_mem(u32 s, u32 bytes)
+static void zero_mem(uint32_t s, uint32_t bytes)
 {
     while ( bytes-- )
         *(char *)s++ = 0;
 }
 
-static u32 copy_mem(u32 src, u32 bytes)
+static uint32_t copy_mem(uint32_t src, uint32_t bytes)
 {
-    u32 dst, dst_ret;
+    uint32_t dst, dst_ret;
 
     dst = alloc_mem(bytes);
     dst_ret = dst;
@@ -96,9 +96,9 @@  static u32 copy_mem(u32 src, u32 bytes)
     return dst_ret;
 }
 
-static u32 copy_string(u32 src)
+static uint32_t copy_string(uint32_t src)
 {
-    u32 p;
+    uint32_t p;
 
     if ( !src )
         return 0;
@@ -109,7 +109,7 @@  static u32 copy_string(u32 src)
     return copy_mem(src, p - src + 1);
 }
 
-static struct hvm_start_info *pvh_info_reloc(u32 in)
+static struct hvm_start_info *pvh_info_reloc(uint32_t in)
 {
     struct hvm_start_info *out;
 
@@ -139,7 +139,7 @@  static struct hvm_start_info *pvh_info_reloc(u32 in)
     return out;
 }
 
-static multiboot_info_t *mbi_reloc(u32 mbi_in)
+static multiboot_info_t *mbi_reloc(uint32_t mbi_in)
 {
     int i;
     multiboot_info_t *mbi_out;
@@ -192,7 +192,7 @@  static multiboot_info_t *mbi2_reloc(uint32_t mbi_in, uint32_t video_out)
 #ifdef CONFIG_VIDEO
     struct boot_video_info *video = NULL;
 #endif
-    u32 ptr;
+    uint32_t ptr;
     unsigned int i, mod_idx = 0;
 
     ptr = alloc_mem(sizeof(*mbi_out));
@@ -203,8 +203,8 @@  static multiboot_info_t *mbi2_reloc(uint32_t mbi_in, uint32_t video_out)
     ptr = ALIGN_UP(mbi_in + sizeof(*mbi_fix), MULTIBOOT2_TAG_ALIGN);
 
     /* Get the number of modules. */
-    for ( tag = _p(ptr); (u32)tag - mbi_in < mbi_fix->total_size;
-          tag = _p(ALIGN_UP((u32)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) )
+    for ( tag = _p(ptr); (uint32_t)tag - mbi_in < mbi_fix->total_size;
+          tag = _p(ALIGN_UP((uint32_t)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) )
     {
         if ( tag->type == MULTIBOOT2_TAG_TYPE_MODULE )
             ++mbi_out->mods_count;
@@ -228,8 +228,8 @@  static multiboot_info_t *mbi2_reloc(uint32_t mbi_in, uint32_t video_out)
     ptr = ALIGN_UP(mbi_in + sizeof(*mbi_fix), MULTIBOOT2_TAG_ALIGN);
 
     /* Put all needed data into mbi_out. */
-    for ( tag = _p(ptr); (u32)tag - mbi_in < mbi_fix->total_size;
-          tag = _p(ALIGN_UP((u32)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) )
+    for ( tag = _p(ptr); (uint32_t)tag - mbi_in < mbi_fix->total_size;
+          tag = _p(ALIGN_UP((uint32_t)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) )
     {
         switch ( tag->type )
         {
@@ -272,10 +272,10 @@  static multiboot_info_t *mbi2_reloc(uint32_t mbi_in, uint32_t video_out)
                 mmap_dst[i].size = sizeof(*mmap_dst);
                 mmap_dst[i].size -= sizeof(mmap_dst[i].size);
                 /* Now copy a given region data. */
-                mmap_dst[i].base_addr_low = (u32)mmap_src->addr;
-                mmap_dst[i].base_addr_high = (u32)(mmap_src->addr >> 32);
-                mmap_dst[i].length_low = (u32)mmap_src->len;
-                mmap_dst[i].length_high = (u32)(mmap_src->len >> 32);
+                mmap_dst[i].base_addr_low = (uint32_t)mmap_src->addr;
+                mmap_dst[i].base_addr_high = (uint32_t)(mmap_src->addr >> 32);
+                mmap_dst[i].length_low = (uint32_t)mmap_src->len;
+                mmap_dst[i].length_high = (uint32_t)(mmap_src->len >> 32);
                 mmap_dst[i].type = mmap_src->type;
                 mmap_src = _p(mmap_src) + get_mb2_data(tag, mmap, entry_size);
             }