[PATCH 0/9] MISRA C 2012 8.1 rule fixes

[PATCH 0/9] MISRA C 2012 8.1 rule fixes

[Michal Orzel]

This series fixes all the findings for MISRA C 2012 8.1 rule, reported by
cppcheck 2.7 with misra addon, for Arm (arm32/arm64 - target allyesconfig).
Fixing this rule comes down to replacing implicit 'unsigned' with explicit
'unsigned int' type as there are no other violations being part of that rule
in the Xen codebase.

The last three patches contain fixes for files that originated from other
projects like Linux kernel or libfdt, therefore they are rather not applicable
to be fixed in Xen (they can be dropped). Nevertheless they act as a sign to
what should be added to a deviation list.

Some important notes:
Static analyzers are not perfect. Cppcheck generates internal AST error for some
of the files resulting in skipping all the checks. For these files, one need to
manually check if there are no findings.

Recently there was a release of a new Cppcheck 2.8 version. However it contains
a regression bug with misra addon. Therefore do not use that version for now.

Michal Orzel (9):
  xen/arm: Use explicitly specified types
  xen/domain: Use explicitly specified types
  xen/common: Use explicitly specified types
  include/xen: Use explicitly specified types
  include/public: Use explicitly specified types
  xsm/flask: Use explicitly specified types
  common/libfdt: Use explicitly specified types
  common/inflate: Use explicitly specified types
  drivers/acpi: Use explicitly specified types

 xen/arch/arm/domain_build.c             |   2 +-
 xen/arch/arm/guestcopy.c                |  13 +-
 xen/arch/arm/include/asm/arm32/bitops.h |   8 +-
 xen/arch/arm/include/asm/fixmap.h       |   4 +-
 xen/arch/arm/include/asm/guest_access.h |   8 +-
 xen/arch/arm/include/asm/mm.h           |   2 +-
 xen/arch/arm/irq.c                      |   2 +-
 xen/arch/arm/kernel.c                   |   2 +-
 xen/arch/arm/mm.c                       |   4 +-
 xen/common/domain.c                     |   2 +-
 xen/common/grant_table.c                |   6 +-
 xen/common/gunzip.c                     |   8 +-
 xen/common/inflate.c                    | 166 ++++++++++++------------
 xen/common/libfdt/fdt_ro.c              |   4 +-
 xen/common/libfdt/fdt_rw.c              |   2 +-
 xen/common/libfdt/fdt_sw.c              |   2 +-
 xen/common/libfdt/fdt_wip.c             |   2 +-
 xen/common/sched/cpupool.c              |   4 +-
 xen/common/trace.c                      |   2 +-
 xen/drivers/acpi/tables/tbfadt.c        |   2 +-
 xen/drivers/acpi/tables/tbutils.c       |   2 +-
 xen/include/public/physdev.h            |   4 +-
 xen/include/public/sysctl.h             |  10 +-
 xen/include/xen/domain.h                |   2 +-
 xen/include/xen/perfc.h                 |   2 +-
 xen/include/xen/sched.h                 |   6 +-
 xen/xsm/flask/ss/avtab.c                |   2 +-
 27 files changed, 137 insertions(+), 136 deletions(-)

-- 
2.25.1

[PATCH 1/9] xen/arm: Use explicitly specified types

[Michal Orzel]

According to MISRA C 2012 Rule 8.1, types shall be explicitly
specified. Fix all the findings reported by cppcheck with misra addon
by substituting implicit type 'unsigned' to explicit 'unsigned int'.

Signed-off-by: Michal Orzel <michal.orzel@arm.com>
---
 xen/arch/arm/domain_build.c             |  2 +-
 xen/arch/arm/guestcopy.c                | 13 +++++++------
 xen/arch/arm/include/asm/arm32/bitops.h |  8 ++++----
 xen/arch/arm/include/asm/fixmap.h       |  4 ++--
 xen/arch/arm/include/asm/guest_access.h |  8 ++++----
 xen/arch/arm/include/asm/mm.h           |  2 +-
 xen/arch/arm/irq.c                      |  2 +-
 xen/arch/arm/kernel.c                   |  2 +-
 xen/arch/arm/mm.c                       |  4 ++--
 9 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 7ddd16c26d..3fd1186b53 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1007,7 +1007,7 @@ static void __init set_interrupt(gic_interrupt_t interrupt,
  */
 static int __init fdt_property_interrupts(const struct kernel_info *kinfo,
                                           gic_interrupt_t *intr,
-                                          unsigned num_irq)
+                                          unsigned int num_irq)
 {
     int res;
 
diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
index 32681606d8..abb6236e27 100644
--- a/xen/arch/arm/guestcopy.c
+++ b/xen/arch/arm/guestcopy.c
@@ -56,7 +56,7 @@ static unsigned long copy_guest(void *buf, uint64_t addr, unsigned int len,
                                 copy_info_t info, unsigned int flags)
 {
     /* XXX needs to handle faults */
-    unsigned offset = addr & ~PAGE_MASK;
+    unsigned int offset = addr & ~PAGE_MASK;
 
     BUILD_BUG_ON((sizeof(addr)) < sizeof(vaddr_t));
     BUILD_BUG_ON((sizeof(addr)) < sizeof(paddr_t));
@@ -64,7 +64,7 @@ static unsigned long copy_guest(void *buf, uint64_t addr, unsigned int len,
     while ( len )
     {
         void *p;
-        unsigned size = min(len, (unsigned)PAGE_SIZE - offset);
+        unsigned int size = min(len, (unsigned int)PAGE_SIZE - offset);
         struct page_info *page;
 
         page = translate_get_page(info, addr, flags & COPY_linear,
@@ -106,26 +106,27 @@ static unsigned long copy_guest(void *buf, uint64_t addr, unsigned int len,
     return 0;
 }
 
-unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len)
+unsigned long raw_copy_to_guest(void *to, const void *from, unsigned int len)
 {
     return copy_guest((void *)from, (vaddr_t)to, len,
                       GVA_INFO(current), COPY_to_guest | COPY_linear);
 }
 
 unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from,
-                                             unsigned len)
+                                             unsigned int len)
 {
     return copy_guest((void *)from, (vaddr_t)to, len, GVA_INFO(current),
                       COPY_to_guest | COPY_flush_dcache | COPY_linear);
 }
 
-unsigned long raw_clear_guest(void *to, unsigned len)
+unsigned long raw_clear_guest(void *to, unsigned int len)
 {
     return copy_guest(NULL, (vaddr_t)to, len, GVA_INFO(current),
                       COPY_to_guest | COPY_linear);
 }
 
-unsigned long raw_copy_from_guest(void *to, const void __user *from, unsigned len)
+unsigned long raw_copy_from_guest(void *to, const void __user *from,
+                                  unsigned int len)
 {
     return copy_guest(to, (vaddr_t)from, len, GVA_INFO(current),
                       COPY_from_guest | COPY_linear);
diff --git a/xen/arch/arm/include/asm/arm32/bitops.h b/xen/arch/arm/include/asm/arm32/bitops.h
index 57938a5874..d0309d47c1 100644
--- a/xen/arch/arm/include/asm/arm32/bitops.h
+++ b/xen/arch/arm/include/asm/arm32/bitops.h
@@ -6,17 +6,17 @@
 /*
  * Little endian assembly bitops.  nr = 0 -> byte 0 bit 0.
  */
-extern int _find_first_zero_bit_le(const void * p, unsigned size);
+extern int _find_first_zero_bit_le(const void * p, unsigned int size);
 extern int _find_next_zero_bit_le(const void * p, int size, int offset);
-extern int _find_first_bit_le(const unsigned long *p, unsigned size);
+extern int _find_first_bit_le(const unsigned long *p, unsigned int size);
 extern int _find_next_bit_le(const unsigned long *p, int size, int offset);
 
 /*
  * Big endian assembly bitops.  nr = 0 -> byte 3 bit 0.
  */
-extern int _find_first_zero_bit_be(const void * p, unsigned size);
+extern int _find_first_zero_bit_be(const void * p, unsigned int size);
 extern int _find_next_zero_bit_be(const void * p, int size, int offset);
-extern int _find_first_bit_be(const unsigned long *p, unsigned size);
+extern int _find_first_bit_be(const unsigned long *p, unsigned int size);
 extern int _find_next_bit_be(const unsigned long *p, int size, int offset);
 
 #ifndef __ARMEB__
diff --git a/xen/arch/arm/include/asm/fixmap.h b/xen/arch/arm/include/asm/fixmap.h
index 365a2385a0..d0c9a52c8c 100644
--- a/xen/arch/arm/include/asm/fixmap.h
+++ b/xen/arch/arm/include/asm/fixmap.h
@@ -30,9 +30,9 @@
 extern lpae_t xen_fixmap[XEN_PT_LPAE_ENTRIES];
 
 /* Map a page in a fixmap entry */
-extern void set_fixmap(unsigned map, mfn_t mfn, unsigned attributes);
+extern void set_fixmap(unsigned int map, mfn_t mfn, unsigned int attributes);
 /* Remove a mapping from a fixmap entry */
-extern void clear_fixmap(unsigned map);
+extern void clear_fixmap(unsigned int map);
 
 #define fix_to_virt(slot) ((void *)FIXMAP_ADDR(slot))
 
diff --git a/xen/arch/arm/include/asm/guest_access.h b/xen/arch/arm/include/asm/guest_access.h
index 53766386d3..4421e43611 100644
--- a/xen/arch/arm/include/asm/guest_access.h
+++ b/xen/arch/arm/include/asm/guest_access.h
@@ -4,11 +4,11 @@
 #include <xen/errno.h>
 #include <xen/sched.h>
 
-unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len);
+unsigned long raw_copy_to_guest(void *to, const void *from, unsigned int len);
 unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from,
-                                             unsigned len);
-unsigned long raw_copy_from_guest(void *to, const void *from, unsigned len);
-unsigned long raw_clear_guest(void *to, unsigned len);
+                                             unsigned int len);
+unsigned long raw_copy_from_guest(void *to, const void *from, unsigned int len);
+unsigned long raw_clear_guest(void *to, unsigned int len);
 
 /* Copy data to guest physical address, then clean the region. */
 unsigned long copy_to_guest_phys_flush_dcache(struct domain *d,
diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
index 045a8ba4bb..c4bc3cd1e5 100644
--- a/xen/arch/arm/include/asm/mm.h
+++ b/xen/arch/arm/include/asm/mm.h
@@ -192,7 +192,7 @@ extern void setup_xenheap_mappings(unsigned long base_mfn, unsigned long nr_mfns
 /* Map a frame table to cover physical addresses ps through pe */
 extern void setup_frametable_mappings(paddr_t ps, paddr_t pe);
 /* map a physical range in virtual memory */
-void __iomem *ioremap_attr(paddr_t start, size_t len, unsigned attributes);
+void __iomem *ioremap_attr(paddr_t start, size_t len, unsigned int attributes);
 
 static inline void __iomem *ioremap_nocache(paddr_t start, size_t len)
 {
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index b761d90c40..b6449c9065 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -610,7 +610,7 @@ void pirq_set_affinity(struct domain *d, int pirq, const cpumask_t *mask)
     BUG();
 }
 
-static bool irq_validate_new_type(unsigned int curr, unsigned new)
+static bool irq_validate_new_type(unsigned int curr, unsigned int new)
 {
     return (curr == IRQ_TYPE_INVALID || curr == new );
 }
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 25ded1c056..2556a45c38 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -256,7 +256,7 @@ static __init int kernel_decompress(struct bootmodule *mod)
     char *output, *input;
     char magic[2];
     int rc;
-    unsigned kernel_order_out;
+    unsigned int kernel_order_out;
     paddr_t output_size;
     struct page_info *pages;
     mfn_t mfn;
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index be37176a47..009b8cd9ef 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -352,7 +352,7 @@ lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned int attr)
 }
 
 /* Map a 4k page in a fixmap entry */
-void set_fixmap(unsigned map, mfn_t mfn, unsigned int flags)
+void set_fixmap(unsigned int map, mfn_t mfn, unsigned int flags)
 {
     int res;
 
@@ -361,7 +361,7 @@ void set_fixmap(unsigned map, mfn_t mfn, unsigned int flags)
 }
 
 /* Remove a mapping from a fixmap entry */
-void clear_fixmap(unsigned map)
+void clear_fixmap(unsigned int map)
 {
     int res;
 
-- 
2.25.1

[PATCH 2/9] xen/domain: Use explicitly specified types

[Michal Orzel]

According to MISRA C 2012 Rule 8.1, types shall be explicitly
specified. Fix all the findings reported by cppcheck with misra addon
by substituting implicit type 'unsigned' to explicit 'unsigned int'.

Signed-off-by: Michal Orzel <michal.orzel@arm.com>
---
 xen/common/domain.c      | 2 +-
 xen/include/xen/domain.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 7570eae91a..57a8515f21 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1446,7 +1446,7 @@ int vcpu_reset(struct vcpu *v)
  * of memory, and it sets a pending event to make sure that a pending
  * event doesn't get missed.
  */
-int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned offset)
+int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned int offset)
 {
     struct domain *d = v->domain;
     void *mapping;
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 1c3c88a14d..628b14b086 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -65,7 +65,7 @@ void cf_check free_pirq_struct(void *);
 int  arch_vcpu_create(struct vcpu *v);
 void arch_vcpu_destroy(struct vcpu *v);
 
-int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned offset);
+int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned int offset);
 void unmap_vcpu_info(struct vcpu *v);
 
 int arch_domain_create(struct domain *d,
-- 
2.25.1

[PATCH 3/9] xen/common: Use explicitly specified types

[Michal Orzel]

According to MISRA C 2012 Rule 8.1, types shall be explicitly
specified. Fix all the findings reported by cppcheck with misra addon
by substituting implicit type 'unsigned' to explicit 'unsigned int'.

Signed-off-by: Michal Orzel <michal.orzel@arm.com>
---
 xen/common/grant_table.c   | 6 +++---
 xen/common/gunzip.c        | 8 ++++----
 xen/common/sched/cpupool.c | 4 ++--
 xen/common/trace.c         | 2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 3918e6de6b..2d110d9f41 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -895,7 +895,7 @@ done:
 static int _set_status(const grant_entry_header_t *shah,
                        grant_status_t *status,
                        struct domain *rd,
-                       unsigned rgt_version,
+                       unsigned int rgt_version,
                        struct active_grant_entry *act,
                        int readonly,
                        int mapflag,
@@ -1763,8 +1763,8 @@ static int
 gnttab_populate_status_frames(struct domain *d, struct grant_table *gt,
                               unsigned int req_nr_frames)
 {
-    unsigned i;
-    unsigned req_status_frames;
+    unsigned int i;
+    unsigned int req_status_frames;
 
     req_status_frames = grant_to_status_frames(req_nr_frames);
 
diff --git a/xen/common/gunzip.c b/xen/common/gunzip.c
index b9ecc17e44..244f8d8903 100644
--- a/xen/common/gunzip.c
+++ b/xen/common/gunzip.c
@@ -13,13 +13,13 @@ static memptr __initdata free_mem_end_ptr;
 #define WSIZE           0x80000000
 
 static unsigned char *__initdata inbuf;
-static unsigned __initdata insize;
+static unsigned int __initdata insize;
 
 /* Index of next byte to be processed in inbuf: */
-static unsigned __initdata inptr;
+static unsigned int __initdata inptr;
 
 /* Bytes in output buffer: */
-static unsigned __initdata outcnt;
+static unsigned int __initdata outcnt;
 
 #define OF(args)        args
 
@@ -72,7 +72,7 @@ static __init void flush_window(void)
      * compute the crc.
      */
     unsigned long c = crc;
-    unsigned n;
+    unsigned int n;
     unsigned char *in, ch;
 
     in = window;
diff --git a/xen/common/sched/cpupool.c b/xen/common/sched/cpupool.c
index a20e3a5fcb..2afe54f54d 100644
--- a/xen/common/sched/cpupool.c
+++ b/xen/common/sched/cpupool.c
@@ -850,7 +850,7 @@ int cpupool_do_sysctl(struct xen_sysctl_cpupool_op *op)
 
     case XEN_SYSCTL_CPUPOOL_OP_ADDCPU:
     {
-        unsigned cpu;
+        unsigned int cpu;
         const cpumask_t *cpus;
 
         cpu = op->cpu;
@@ -895,7 +895,7 @@ int cpupool_do_sysctl(struct xen_sysctl_cpupool_op *op)
 
     case XEN_SYSCTL_CPUPOOL_OP_RMCPU:
     {
-        unsigned cpu;
+        unsigned int cpu;
 
         c = cpupool_get_by_id(op->cpupool_id);
         ret = -ENOENT;
diff --git a/xen/common/trace.c b/xen/common/trace.c
index a7c092fcbb..fb3752ce62 100644
--- a/xen/common/trace.c
+++ b/xen/common/trace.c
@@ -834,7 +834,7 @@ void __trace_hypercall(uint32_t event, unsigned long op,
 
 #define APPEND_ARG32(i)                         \
     do {                                        \
-        unsigned i_ = (i);                      \
+        unsigned int i_ = (i);                  \
         *a++ = args[(i_)];                      \
         d.op |= TRC_PV_HYPERCALL_V2_ARG_32(i_); \
     } while( 0 )
-- 
2.25.1

[PATCH 4/9] include/xen: Use explicitly specified types

[Michal Orzel]

According to MISRA C 2012 Rule 8.1, types shall be explicitly
specified. Fix all the findings reported by cppcheck with misra addon
by substituting implicit type 'unsigned' to explicit 'unsigned int'.

Signed-off-by: Michal Orzel <michal.orzel@arm.com>
---
 xen/include/xen/perfc.h | 2 +-
 xen/include/xen/sched.h | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/include/xen/perfc.h b/xen/include/xen/perfc.h
index bb010b0aae..7c5ce537bd 100644
--- a/xen/include/xen/perfc.h
+++ b/xen/include/xen/perfc.h
@@ -49,7 +49,7 @@ enum perfcounter {
 #undef PERFSTATUS
 #undef PERFSTATUS_ARRAY
 
-typedef unsigned perfc_t;
+typedef unsigned int perfc_t;
 #define PRIperfc ""
 
 DECLARE_PER_CPU(perfc_t[NUM_PERFCOUNTERS], perfcounters);
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 463d41ffb6..d957b6e11f 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -518,9 +518,9 @@ struct domain
 
     /* hvm_print_line() and guest_console_write() logging. */
 #define DOMAIN_PBUF_SIZE 200
-    char       *pbuf;
-    unsigned    pbuf_idx;
-    spinlock_t  pbuf_lock;
+    char         *pbuf;
+    unsigned int  pbuf_idx;
+    spinlock_t    pbuf_lock;
 
     /* OProfile support. */
     struct xenoprof *xenoprof;
-- 
2.25.1

[PATCH 5/9] include/public: Use explicitly specified types

[Michal Orzel]

According to MISRA C 2012 Rule 8.1, types shall be explicitly
specified. Fix all the findings reported by cppcheck with misra addon
by substituting implicit type 'unsigned' to explicit 'unsigned int'.

Bump sysctl interface version.

Signed-off-by: Michal Orzel <michal.orzel@arm.com>
---
 xen/include/public/physdev.h |  4 ++--
 xen/include/public/sysctl.h  | 10 +++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/xen/include/public/physdev.h b/xen/include/public/physdev.h
index d271766ad0..a2ca0ee564 100644
--- a/xen/include/public/physdev.h
+++ b/xen/include/public/physdev.h
@@ -211,8 +211,8 @@ struct physdev_manage_pci_ext {
     /* IN */
     uint8_t bus;
     uint8_t devfn;
-    unsigned is_extfn;
-    unsigned is_virtfn;
+    unsigned int is_extfn;
+    unsigned int is_virtfn;
     struct {
         uint8_t bus;
         uint8_t devfn;
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index b0a4af8789..a2a762fe46 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -35,7 +35,7 @@
 #include "domctl.h"
 #include "physdev.h"
 
-#define XEN_SYSCTL_INTERFACE_VERSION 0x00000014
+#define XEN_SYSCTL_INTERFACE_VERSION 0x00000015
 
 /*
  * Read console content from Xen buffer ring.
@@ -644,18 +644,18 @@ struct xen_sysctl_credit_schedule {
     /* Length of timeslice in milliseconds */
 #define XEN_SYSCTL_CSCHED_TSLICE_MAX 1000
 #define XEN_SYSCTL_CSCHED_TSLICE_MIN 1
-    unsigned tslice_ms;
-    unsigned ratelimit_us;
+    unsigned int tslice_ms;
+    unsigned int ratelimit_us;
     /*
      * How long we consider a vCPU to be cache-hot on the
      * CPU where it has run (max 100ms, in microseconds)
     */
 #define XEN_SYSCTL_CSCHED_MGR_DLY_MAX_US (100 * 1000)
-    unsigned vcpu_migr_delay_us;
+    unsigned int vcpu_migr_delay_us;
 };
 
 struct xen_sysctl_credit2_schedule {
-    unsigned ratelimit_us;
+    unsigned int ratelimit_us;
 };
 
 /* XEN_SYSCTL_scheduler_op */
-- 
2.25.1

[PATCH 6/9] xsm/flask: Use explicitly specified types

[Michal Orzel]

According to MISRA C 2012 Rule 8.1, types shall be explicitly
specified. Fix all the findings reported by cppcheck with misra addon
by substituting implicit type 'unsigned' to explicit 'unsigned int'.

Signed-off-by: Michal Orzel <michal.orzel@arm.com>
---
 xen/xsm/flask/ss/avtab.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/xsm/flask/ss/avtab.c b/xen/xsm/flask/ss/avtab.c
index 017f5183de..9761d028d8 100644
--- a/xen/xsm/flask/ss/avtab.c
+++ b/xen/xsm/flask/ss/avtab.c
@@ -349,7 +349,7 @@ int avtab_read_item(struct avtab *a, void *fp, struct policydb *pol,
     struct avtab_key key;
     struct avtab_datum datum;
     int i, rc;
-    unsigned set;
+    unsigned int set;
 
     memset(&key, 0, sizeof(struct avtab_key));
     memset(&datum, 0, sizeof(struct avtab_datum));
-- 
2.25.1

[PATCH 7/9] common/libfdt: Use explicitly specified types

[Michal Orzel]

According to MISRA C 2012 Rule 8.1, types shall be explicitly
specified. Fix all the findings reported by cppcheck with misra addon
by substituting implicit type 'unsigned' to explicit 'unsigned int'.

Signed-off-by: Michal Orzel <michal.orzel@arm.com>
---
This patch may not be applicable as these files come from libfdt.
---
 xen/common/libfdt/fdt_ro.c  | 4 ++--
 xen/common/libfdt/fdt_rw.c  | 2 +-
 xen/common/libfdt/fdt_sw.c  | 2 +-
 xen/common/libfdt/fdt_wip.c | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/xen/common/libfdt/fdt_ro.c b/xen/common/libfdt/fdt_ro.c
index 17584da257..0fc4f793fe 100644
--- a/xen/common/libfdt/fdt_ro.c
+++ b/xen/common/libfdt/fdt_ro.c
@@ -53,7 +53,7 @@ const char *fdt_get_string(const void *fdt, int stroffset, int *lenp)
 
 	err = -FDT_ERR_BADOFFSET;
 	absoffset = stroffset + fdt_off_dt_strings(fdt);
-	if (absoffset >= (unsigned)totalsize)
+	if (absoffset >= (unsigned int)totalsize)
 		goto fail;
 	len = totalsize - absoffset;
 
@@ -61,7 +61,7 @@ const char *fdt_get_string(const void *fdt, int stroffset, int *lenp)
 		if (stroffset < 0)
 			goto fail;
 		if (can_assume(LATEST) || fdt_version(fdt) >= 17) {
-			if ((unsigned)stroffset >= fdt_size_dt_strings(fdt))
+			if ((unsigned int)stroffset >= fdt_size_dt_strings(fdt))
 				goto fail;
 			if ((fdt_size_dt_strings(fdt) - stroffset) < len)
 				len = fdt_size_dt_strings(fdt) - stroffset;
diff --git a/xen/common/libfdt/fdt_rw.c b/xen/common/libfdt/fdt_rw.c
index 3621d3651d..1707238ebc 100644
--- a/xen/common/libfdt/fdt_rw.c
+++ b/xen/common/libfdt/fdt_rw.c
@@ -59,7 +59,7 @@ static int fdt_splice_(void *fdt, void *splicepoint, int oldlen, int newlen)
 
 	if ((oldlen < 0) || (soff + oldlen < soff) || (soff + oldlen > dsize))
 		return -FDT_ERR_BADOFFSET;
-	if ((p < (char *)fdt) || (dsize + newlen < (unsigned)oldlen))
+	if ((p < (char *)fdt) || (dsize + newlen < (unsigned int)oldlen))
 		return -FDT_ERR_BADOFFSET;
 	if (dsize - oldlen + newlen > fdt_totalsize(fdt))
 		return -FDT_ERR_NOSPACE;
diff --git a/xen/common/libfdt/fdt_sw.c b/xen/common/libfdt/fdt_sw.c
index 4c569ee7eb..eb694b5dbb 100644
--- a/xen/common/libfdt/fdt_sw.c
+++ b/xen/common/libfdt/fdt_sw.c
@@ -162,7 +162,7 @@ int fdt_resize(void *fdt, void *buf, int bufsize)
 	    headsize + tailsize > fdt_totalsize(fdt))
 		return -FDT_ERR_INTERNAL;
 
-	if ((headsize + tailsize) > (unsigned)bufsize)
+	if ((headsize + tailsize) > (unsigned int)bufsize)
 		return -FDT_ERR_NOSPACE;
 
 	oldtail = (char *)fdt + fdt_totalsize(fdt) - tailsize;
diff --git a/xen/common/libfdt/fdt_wip.c b/xen/common/libfdt/fdt_wip.c
index c2d7566a67..82db674014 100644
--- a/xen/common/libfdt/fdt_wip.c
+++ b/xen/common/libfdt/fdt_wip.c
@@ -23,7 +23,7 @@ int fdt_setprop_inplace_namelen_partial(void *fdt, int nodeoffset,
 	if (!propval)
 		return proplen;
 
-	if ((unsigned)proplen < (len + idx))
+	if ((unsigned int)proplen < (len + idx))
 		return -FDT_ERR_NOSPACE;
 
 	memcpy((char *)propval + idx, val, len);
-- 
2.25.1

[PATCH 8/9] common/inflate: Use explicitly specified types

[Michal Orzel]

According to MISRA C 2012 Rule 8.1, types shall be explicitly
specified. Fix all the findings reported by cppcheck with misra addon
by substituting implicit type 'unsigned' to explicit 'unsigned int'.

Signed-off-by: Michal Orzel <michal.orzel@arm.com>
---
This patch may not be applicable as inflate comes from Linux.
---
 xen/common/inflate.c | 166 +++++++++++++++++++++----------------------
 1 file changed, 83 insertions(+), 83 deletions(-)

diff --git a/xen/common/inflate.c b/xen/common/inflate.c
index 8fa4b96d12..71616ff60c 100644
--- a/xen/common/inflate.c
+++ b/xen/common/inflate.c
@@ -138,7 +138,7 @@ struct huft {
 
 
 /* Function prototypes */
-static int huft_build OF((unsigned *, unsigned, unsigned,
+static int huft_build OF((unsigned int *, unsigned int, unsigned int,
                           const ush *, const ush *, struct huft **, int *));
 static int huft_free OF((struct huft *));
 static int inflate_codes OF((struct huft *, struct huft *, int, int));
@@ -162,20 +162,20 @@ static int inflate OF((void));
 #define flush_output(w) (wp=(w),flush_window())
 
 /* Tables for deflate from PKZIP's appnote.txt. */
-static const unsigned border[] = {    /* Order of the bit length code lengths */
+static const unsigned int border[] = { /* Order of the bit length code lengths */
     16, 17, 18, 0, 8, 7, 9, 6, 10, 5, 11, 4, 12, 3, 13, 2, 14, 1, 15};
-static const ush cplens[] = {         /* Copy lengths for literal codes 257..285 */
+static const ush cplens[] = {          /* Copy lengths for literal codes 257..285 */
     3, 4, 5, 6, 7, 8, 9, 10, 11, 13, 15, 17, 19, 23, 27, 31,
     35, 43, 51, 59, 67, 83, 99, 115, 131, 163, 195, 227, 258, 0, 0};
 /* note: see note #13 above about the 258 in this list. */
-static const ush cplext[] = {         /* Extra bits for literal codes 257..285 */
+static const ush cplext[] = {          /* Extra bits for literal codes 257..285 */
     0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 2, 2, 2, 2,
     3, 3, 3, 3, 4, 4, 4, 4, 5, 5, 5, 5, 0, 99, 99}; /* 99==invalid */
-static const ush cpdist[] = {         /* Copy offsets for distance codes 0..29 */
+static const ush cpdist[] = {          /* Copy offsets for distance codes 0..29 */
     1, 2, 3, 4, 5, 7, 9, 13, 17, 25, 33, 49, 65, 97, 129, 193,
     257, 385, 513, 769, 1025, 1537, 2049, 3073, 4097, 6145,
     8193, 12289, 16385, 24577};
-static const ush cpdext[] = {         /* Extra bits for distance codes */
+static const ush cpdext[] = {          /* Extra bits for distance codes */
     0, 0, 0, 0, 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6,
     7, 7, 8, 8, 9, 9, 10, 10, 11, 11,
     12, 12, 13, 13};
@@ -213,7 +213,7 @@ static const ush cpdext[] = {         /* Extra bits for distance codes */
  */
 
 static ulg __initdata bb;                /* bit buffer */
-static unsigned __initdata bk;           /* bits in bit buffer */
+static unsigned int __initdata bk;       /* bits in bit buffer */
 
 static const ush mask_bits[] = {
     0x0000,
@@ -313,13 +313,13 @@ static const int dbits = 6;          /* bits in base distance lookup table */
 #define N_MAX 288       /* maximum number of codes in any set */
 
 
-static unsigned __initdata hufts;      /* track memory usage */
+static unsigned int __initdata hufts;      /* track memory usage */
 
 
 static int __init huft_build(
-    unsigned *b,            /* code lengths in bits (all assumed <= BMAX) */
-    unsigned n,             /* number of codes (assumed <= N_MAX) */
-    unsigned s,             /* number of simple-valued codes (0..s-1) */
+    unsigned int *b,        /* code lengths in bits (all assumed <= BMAX) */
+    unsigned int n,         /* number of codes (assumed <= N_MAX) */
+    unsigned int s,         /* number of simple-valued codes (0..s-1) */
     const ush *d,           /* list of base values for non-simple codes */
     const ush *e,           /* list of extra bits for non-simple codes */
     struct huft **t,        /* result: starting table */
@@ -331,28 +331,28 @@ static int __init huft_build(
    case), two if the input is invalid (all zero length codes or an
    oversubscribed set of lengths), and three if not enough memory. */
 {
-    unsigned a;                   /* counter for codes of length k */
-    unsigned f;                   /* i repeats in table every f entries */
+    unsigned int a;               /* counter for codes of length k */
+    unsigned int f;               /* i repeats in table every f entries */
     int g;                        /* maximum code length */
     int h;                        /* table level */
-    register unsigned i;          /* counter, current code */
-    register unsigned j;          /* counter */
+    register unsigned int i;      /* counter, current code */
+    register unsigned int j;      /* counter */
     register int k;               /* number of bits in current code */
     int l;                        /* bits per table (returned in m) */
-    register unsigned *p;         /* pointer into c[], b[], or v[] */
+    register unsigned int *p;     /* pointer into c[], b[], or v[] */
     register struct huft *q;      /* points to current table */
     struct huft r;                /* table entry for structure assignment */
     register int w;               /* bits before this table == (l * h) */
-    unsigned *xp;                 /* pointer into x */
+    unsigned int *xp;             /* pointer into x */
     int y;                        /* number of dummy codes added */
-    unsigned z;                   /* number of entries in current table */
+    unsigned int z;               /* number of entries in current table */
     struct {
-        unsigned c[BMAX+1];           /* bit length count table */
-        struct huft *u[BMAX];         /* table stack */
-        unsigned v[N_MAX];            /* values in order of bit length */
-        unsigned x[BMAX+1];           /* bit offsets, then code stack */
+        unsigned int c[BMAX+1];   /* bit length count table */
+        struct huft *u[BMAX];     /* table stack */
+        unsigned int v[N_MAX];    /* values in order of bit length */
+        unsigned int x[BMAX+1];   /* bit offsets, then code stack */
     } *stk;
-    unsigned *c, *v, *x;
+    unsigned int *c, *v, *x;
     struct huft **u;
     int ret;
 
@@ -392,13 +392,13 @@ static int __init huft_build(
         if (c[j])
             break;
     k = j;                        /* minimum code length */
-    if ((unsigned)l < j)
+    if ((unsigned int)l < j)
         l = j;
     for (i = BMAX; i; i--)
         if (c[i])
             break;
     g = i;                        /* maximum code length */
-    if ((unsigned)l > i)
+    if ((unsigned int)l > i)
         l = i;
     *m = l;
 
@@ -464,7 +464,7 @@ static int __init huft_build(
                 w += l;                 /* previous table always l bits */
 
                 /* compute minimum size table less than or equal to l bits */
-                z = (z = g - w) > (unsigned)l ? l : z;  /* upper limit on table size */
+                z = (z = g - w) > (unsigned int)l ? l : z;  /* upper limit on table size */
                 if ((f = 1 << (j = k - w)) > a + 1)     /* try a k-w bit table */
                 {                       /* too few codes for k-w bit table */
                     DEBG1("2 ");
@@ -592,13 +592,13 @@ static int __init inflate_codes(
 /* inflate (decompress) the codes in a deflated (compressed) block.
    Return an error code or zero if it all goes ok. */
 {
-    register unsigned e;  /* table entry flag/number of extra bits */
-    unsigned n, d;        /* length and index for copy */
-    unsigned w;           /* current window position */
-    struct huft *t;       /* pointer to table entry */
-    unsigned ml, md;      /* masks for bl and bd bits */
-    register ulg b;       /* bit buffer */
-    register unsigned k;  /* number of bits in bit buffer */
+    register unsigned int e;  /* table entry flag/number of extra bits */
+    unsigned int n, d;        /* length and index for copy */
+    unsigned int w;           /* current window position */
+    struct huft *t;           /* pointer to table entry */
+    unsigned int ml, md;      /* masks for bl and bd bits */
+    register ulg b;           /* bit buffer */
+    register unsigned int k;  /* number of bits in bit buffer */
 
 
     /* make local copies of globals */
@@ -611,15 +611,15 @@ static int __init inflate_codes(
     md = mask_bits[bd];
     for (;;)                      /* do until end of block */
     {
-        NEEDBITS((unsigned)bl)
-            if ((e = (t = tl + ((unsigned)b & ml))->e) > 16)
+        NEEDBITS((unsigned int)bl)
+            if ((e = (t = tl + ((unsigned int)b & ml))->e) > 16)
                 do {
                     if (e == 99)
                         return 1;
                     DUMPBITS(t->b)
                         e -= 16;
                     NEEDBITS(e)
-                        } while ((e = (t = t->v.t + ((unsigned)b & mask_bits[e]))->e) > 16);
+                        } while ((e = (t = t->v.t + ((unsigned int)b & mask_bits[e]))->e) > 16);
         DUMPBITS(t->b)
             if (e == 16)                /* then it's a literal */
             {
@@ -639,22 +639,22 @@ static int __init inflate_codes(
 
                 /* get length of block to copy */
                 NEEDBITS(e)
-                    n = t->v.n + ((unsigned)b & mask_bits[e]);
+                    n = t->v.n + ((unsigned int)b & mask_bits[e]);
                 DUMPBITS(e);
 
                 /* decode distance of block to copy */
-                NEEDBITS((unsigned)bd)
-                    if ((e = (t = td + ((unsigned)b & md))->e) > 16)
+                NEEDBITS((unsigned int)bd)
+                    if ((e = (t = td + ((unsigned int)b & md))->e) > 16)
                         do {
                             if (e == 99)
                                 return 1;
                             DUMPBITS(t->b)
                                 e -= 16;
                             NEEDBITS(e)
-                                } while ((e = (t = t->v.t + ((unsigned)b & mask_bits[e]))->e) > 16);
+                                } while ((e = (t = t->v.t + ((unsigned int)b & mask_bits[e]))->e) > 16);
                 DUMPBITS(t->b)
                     NEEDBITS(e)
-                    d = w - t->v.n - ((unsigned)b & mask_bits[e]);
+                    d = w - t->v.n - ((unsigned int)b & mask_bits[e]);
                 DUMPBITS(e)
                     Tracevv((stderr,"\\[%d,%d]", w-d, n));
 
@@ -701,10 +701,10 @@ static int __init inflate_codes(
 static int __init inflate_stored(void)
 /* "decompress" an inflated type 0 (stored) block. */
 {
-    unsigned n;           /* number of bytes in block */
-    unsigned w;           /* current window position */
-    register ulg b;       /* bit buffer */
-    register unsigned k;  /* number of bits in bit buffer */
+    unsigned int n;           /* number of bytes in block */
+    unsigned int w;           /* current window position */
+    register ulg b;           /* bit buffer */
+    register unsigned int k;  /* number of bits in bit buffer */
 
     DEBG("<stor");
 
@@ -721,10 +721,10 @@ static int __init inflate_stored(void)
 
     /* get the length and its complement */
     NEEDBITS(16)
-        n = ((unsigned)b & 0xffff);
+        n = ((unsigned int)b & 0xffff);
     DUMPBITS(16)
         NEEDBITS(16)
-        if (n != (unsigned)((~b) & 0xffff))
+        if (n != (unsigned int)((~b) & 0xffff))
             return 1;                   /* error in compressed data */
     DUMPBITS(16)
 
@@ -769,7 +769,7 @@ static int noinline __init inflate_fixed(void)
     struct huft *td;      /* distance code table */
     int bl;               /* lookup bits for tl */
     int bd;               /* lookup bits for td */
-    unsigned *l;          /* length list for huft_build */
+    unsigned int *l;      /* length list for huft_build */
 
     DEBG("<fix");
 
@@ -826,21 +826,21 @@ static int noinline __init inflate_fixed(void)
 static int noinline __init inflate_dynamic(void)
 /* decompress an inflated type 2 (dynamic Huffman codes) block. */
 {
-    int i;                /* temporary variables */
-    unsigned j;
-    unsigned l;           /* last length */
-    unsigned m;           /* mask for bit lengths table */
-    unsigned n;           /* number of lengths to get */
-    struct huft *tl;      /* literal/length code table */
-    struct huft *td;      /* distance code table */
-    int bl;               /* lookup bits for tl */
-    int bd;               /* lookup bits for td */
-    unsigned nb;          /* number of bit length codes */
-    unsigned nl;          /* number of literal/length codes */
-    unsigned nd;          /* number of distance codes */
-    unsigned *ll;         /* literal/length and distance code lengths */
-    register ulg b;       /* bit buffer */
-    register unsigned k;  /* number of bits in bit buffer */
+    int i;                    /* temporary variables */
+    unsigned int j;
+    unsigned int l;           /* last length */
+    unsigned int m;           /* mask for bit lengths table */
+    unsigned int n;           /* number of lengths to get */
+    struct huft *tl;          /* literal/length code table */
+    struct huft *td;          /* distance code table */
+    int bl;                   /* lookup bits for tl */
+    int bd;                   /* lookup bits for td */
+    unsigned int nb;          /* number of bit length codes */
+    unsigned int nl;          /* number of literal/length codes */
+    unsigned int nd;          /* number of distance codes */
+    unsigned int *ll;         /* literal/length and distance code lengths */
+    register ulg b;           /* bit buffer */
+    register unsigned int k;  /* number of bits in bit buffer */
     int ret;
 
     DEBG("<dyn");
@@ -861,13 +861,13 @@ static int noinline __init inflate_dynamic(void)
 
     /* read in table lengths */
     NEEDBITS(5)
-        nl = 257 + ((unsigned)b & 0x1f);      /* number of literal/length codes */
+        nl = 257 + ((unsigned int)b & 0x1f);      /* number of literal/length codes */
     DUMPBITS(5)
         NEEDBITS(5)
-        nd = 1 + ((unsigned)b & 0x1f);        /* number of distance codes */
+        nd = 1 + ((unsigned int)b & 0x1f);        /* number of distance codes */
     DUMPBITS(5)
         NEEDBITS(4)
-        nb = 4 + ((unsigned)b & 0xf);         /* number of bit length codes */
+        nb = 4 + ((unsigned int)b & 0xf);         /* number of bit length codes */
     DUMPBITS(4)
 #ifdef PKZIP_BUG_WORKAROUND
         if (nl > 288 || nd > 32)
@@ -885,7 +885,7 @@ static int noinline __init inflate_dynamic(void)
     for (j = 0; j < nb; j++)
     {
         NEEDBITS(3)
-            ll[border[j]] = (unsigned)b & 7;
+            ll[border[j]] = (unsigned int)b & 7;
         DUMPBITS(3)
             }
     for (; j < 19; j++)
@@ -909,10 +909,10 @@ static int noinline __init inflate_dynamic(void)
     n = nl + nd;
     m = mask_bits[bl];
     i = l = 0;
-    while ((unsigned)i < n)
+    while ((unsigned int)i < n)
     {
-        NEEDBITS((unsigned)bl)
-            j = (td = tl + ((unsigned)b & m))->b;
+        NEEDBITS((unsigned int)bl)
+            j = (td = tl + ((unsigned int)b & m))->b;
         DUMPBITS(j)
             j = td->v.n;
         if (j < 16)                 /* length of code in bits (0..15) */
@@ -920,9 +920,9 @@ static int noinline __init inflate_dynamic(void)
         else if (j == 16)           /* repeat last length 3 to 6 times */
         {
             NEEDBITS(2)
-                j = 3 + ((unsigned)b & 3);
+                j = 3 + ((unsigned int)b & 3);
             DUMPBITS(2)
-                if ((unsigned)i + j > n) {
+                if ((unsigned int)i + j > n) {
                     ret = 1;
                     goto out;
                 }
@@ -932,9 +932,9 @@ static int noinline __init inflate_dynamic(void)
         else if (j == 17)           /* 3 to 10 zero length codes */
         {
             NEEDBITS(3)
-                j = 3 + ((unsigned)b & 7);
+                j = 3 + ((unsigned int)b & 7);
             DUMPBITS(3)
-                if ((unsigned)i + j > n) {
+                if ((unsigned int)i + j > n) {
                     ret = 1;
                     goto out;
                 }
@@ -945,9 +945,9 @@ static int noinline __init inflate_dynamic(void)
         else                        /* j == 18: 11 to 138 zero length codes */
         {
             NEEDBITS(7)
-                j = 11 + ((unsigned)b & 0x7f);
+                j = 11 + ((unsigned int)b & 0x7f);
             DUMPBITS(7)
-                if ((unsigned)i + j > n) {
+                if ((unsigned int)i + j > n) {
                     ret = 1;
                     goto out;
                 }
@@ -1033,9 +1033,9 @@ int *e                  /* last block flag */
 )
 /* decompress an inflated block */
 {
-unsigned t;           /* block type */
-register ulg b;       /* bit buffer */
-register unsigned k;  /* number of bits in bit buffer */
+unsigned int t;           /* block type */
+register ulg b;           /* bit buffer */
+register unsigned int k;  /* number of bits in bit buffer */
 
 DEBG("<blk");
 
@@ -1052,7 +1052,7 @@ NEEDBITS(1)
 
     /* read in block type */
     NEEDBITS(2)
-    t = (unsigned)b & 3;
+    t = (unsigned int)b & 3;
     DUMPBITS(2)
 
 
@@ -1084,7 +1084,7 @@ static int __init inflate(void)
 {
     int e;                /* last block flag */
     int r;                /* result code */
-    unsigned h;           /* maximum struct huft's malloc'ed */
+    unsigned int h;       /* maximum struct huft's malloc'ed */
 
     /* initialize window, bit buffer */
     wp = 0;
@@ -1235,8 +1235,8 @@ static int __init gunzip(void)
     (void)NEXTBYTE();  /* Ignore OS type for the moment */
 
     if ((flags & EXTRA_FIELD) != 0) {
-        unsigned len = (unsigned)NEXTBYTE();
-        len |= ((unsigned)NEXTBYTE())<<8;
+        unsigned int len = (unsigned int)NEXTBYTE();
+        len |= ((unsigned int)NEXTBYTE())<<8;
         while (len--) (void)NEXTBYTE();
     }
 
-- 
2.25.1

[PATCH 9/9] drivers/acpi: Use explicitly specified types

[Michal Orzel]

According to MISRA C 2012 Rule 8.1, types shall be explicitly
specified. Fix all the findings reported by cppcheck with misra addon
by substituting implicit type 'unsigned' to explicit 'unsigned int'.

Signed-off-by: Michal Orzel <michal.orzel@arm.com>
---
This patch may not be applicable as these files come from Linux.
---
 xen/drivers/acpi/tables/tbfadt.c  | 2 +-
 xen/drivers/acpi/tables/tbutils.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/acpi/tables/tbfadt.c b/xen/drivers/acpi/tables/tbfadt.c
index f11fd5a900..ad55cd769a 100644
--- a/xen/drivers/acpi/tables/tbfadt.c
+++ b/xen/drivers/acpi/tables/tbfadt.c
@@ -235,7 +235,7 @@ void __init acpi_tb_create_local_fadt(struct acpi_table_header *table, u32 lengt
 		ACPI_WARNING((AE_INFO,
 			      "FADT (revision %u) is longer than ACPI 5.0 version,"
 			      " truncating length %u to %zu",
-			      table->revision, (unsigned)length,
+			      table->revision, (unsigned int)length,
 			      sizeof(struct acpi_table_fadt)));
 	}
 
diff --git a/xen/drivers/acpi/tables/tbutils.c b/xen/drivers/acpi/tables/tbutils.c
index d135a50ff9..ddb7f628c9 100644
--- a/xen/drivers/acpi/tables/tbutils.c
+++ b/xen/drivers/acpi/tables/tbutils.c
@@ -481,7 +481,7 @@ acpi_tb_parse_root_table(acpi_physical_address rsdp_address, u8 flags)
 			if (ACPI_FAILURE(status)) {
 				ACPI_WARNING((AE_INFO,
 					      "Truncating %u table entries!",
-					      (unsigned)
+					      (unsigned int)
 					      (acpi_gbl_root_table_list.size -
 					       acpi_gbl_root_table_list.
 					       count)));
-- 
2.25.1

Re: [PATCH 1/9] xen/arm: Use explicitly specified types

[Julien Grall]

Hi Michal,

On 20/06/2022 08:02, Michal Orzel wrote:
> According to MISRA C 2012 Rule 8.1, types shall be explicitly
> specified. Fix all the findings reported by cppcheck with misra addon
> by substituting implicit type 'unsigned' to explicit 'unsigned int'.
> 
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>

Reviewed-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall

Re: [PATCH 2/9] xen/domain: Use explicitly specified types

[Julien Grall]

Hi Michal,

On 20/06/2022 08:02, Michal Orzel wrote:
> According to MISRA C 2012 Rule 8.1, types shall be explicitly
> specified. Fix all the findings reported by cppcheck with misra addon
> by substituting implicit type 'unsigned' to explicit 'unsigned int'.
> 
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>

Reviewed-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall

Re: [PATCH 3/9] xen/common: Use explicitly specified types

[Julien Grall]

Hi Michal,

On 20/06/2022 08:02, Michal Orzel wrote:
> According to MISRA C 2012 Rule 8.1, types shall be explicitly
> specified. Fix all the findings reported by cppcheck with misra addon
> by substituting implicit type 'unsigned' to explicit 'unsigned int'.
> 
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>

Reviewed-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall

Re: [PATCH 3/9] xen/common: Use explicitly specified types

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 369 bytes --]

On 20.06.22 09:02, Michal Orzel wrote:
> According to MISRA C 2012 Rule 8.1, types shall be explicitly
> specified. Fix all the findings reported by cppcheck with misra addon
> by substituting implicit type 'unsigned' to explicit 'unsigned int'.
> 
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH 4/9] include/xen: Use explicitly specified types

[Julien Grall]

Hi,

On 20/06/2022 08:02, Michal Orzel wrote:
> According to MISRA C 2012 Rule 8.1, types shall be explicitly
> specified. Fix all the findings reported by cppcheck with misra addon
> by substituting implicit type 'unsigned' to explicit 'unsigned int'.
> 
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>
> ---
>   xen/include/xen/perfc.h | 2 +-
>   xen/include/xen/sched.h | 6 +++---
>   2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/include/xen/perfc.h b/xen/include/xen/perfc.h
> index bb010b0aae..7c5ce537bd 100644
> --- a/xen/include/xen/perfc.h
> +++ b/xen/include/xen/perfc.h
> @@ -49,7 +49,7 @@ enum perfcounter {
>   #undef PERFSTATUS
>   #undef PERFSTATUS_ARRAY
>   
> -typedef unsigned perfc_t;
> +typedef unsigned int perfc_t;
>   #define PRIperfc ""
>   
>   DECLARE_PER_CPU(perfc_t[NUM_PERFCOUNTERS], perfcounters);
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 463d41ffb6..d957b6e11f 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -518,9 +518,9 @@ struct domain
>   
>       /* hvm_print_line() and guest_console_write() logging. */
>   #define DOMAIN_PBUF_SIZE 200
> -    char       *pbuf;
> -    unsigned    pbuf_idx;
> -    spinlock_t  pbuf_lock;
> +    char         *pbuf;
> +    unsigned int  pbuf_idx;
> +    spinlock_t    pbuf_lock;

Looking a "struct domain", we don't often seem to align the name. In 
fact, this is something I don't particularly like because it adds a lot 
of churn in a patch.

So my preference would be to just change the line "unsigned".

Other than that:

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall

Re: [PATCH 5/9] include/public: Use explicitly specified types

[Julien Grall]

Hi Michal,

On 20/06/2022 08:02, Michal Orzel wrote:
> According to MISRA C 2012 Rule 8.1, types shall be explicitly
> specified. Fix all the findings reported by cppcheck with misra addon
> by substituting implicit type 'unsigned' to explicit 'unsigned int'.
> 
> Bump sysctl interface version.

The sysctl version should only be bumped if the ABI has changed. AFAICT 
switching from "unsigned" to "unsigned" will not modify it, so I don't 
think this is necessary.

> 
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>
> ---
>   xen/include/public/physdev.h |  4 ++--
>   xen/include/public/sysctl.h  | 10 +++++-----
>   2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/include/public/physdev.h b/xen/include/public/physdev.h
> index d271766ad0..a2ca0ee564 100644
> --- a/xen/include/public/physdev.h
> +++ b/xen/include/public/physdev.h
> @@ -211,8 +211,8 @@ struct physdev_manage_pci_ext {
>       /* IN */
>       uint8_t bus;
>       uint8_t devfn;
> -    unsigned is_extfn;
> -    unsigned is_virtfn;
> +    unsigned int is_extfn;
> +    unsigned int is_virtfn;
>       struct {
>           uint8_t bus;
>           uint8_t devfn;
> diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> index b0a4af8789..a2a762fe46 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -35,7 +35,7 @@
>   #include "domctl.h"
>   #include "physdev.h"
>   
> -#define XEN_SYSCTL_INTERFACE_VERSION 0x00000014
> +#define XEN_SYSCTL_INTERFACE_VERSION 0x00000015
>   
>   /*
>    * Read console content from Xen buffer ring.
> @@ -644,18 +644,18 @@ struct xen_sysctl_credit_schedule {
>       /* Length of timeslice in milliseconds */
>   #define XEN_SYSCTL_CSCHED_TSLICE_MAX 1000
>   #define XEN_SYSCTL_CSCHED_TSLICE_MIN 1
> -    unsigned tslice_ms;
> -    unsigned ratelimit_us;
> +    unsigned int tslice_ms;
> +    unsigned int ratelimit_us;
>       /*
>        * How long we consider a vCPU to be cache-hot on the
>        * CPU where it has run (max 100ms, in microseconds)
>       */
>   #define XEN_SYSCTL_CSCHED_MGR_DLY_MAX_US (100 * 1000)
> -    unsigned vcpu_migr_delay_us;
> +    unsigned int vcpu_migr_delay_us;
>   };
>   
>   struct xen_sysctl_credit2_schedule {
> -    unsigned ratelimit_us;
> +    unsigned int ratelimit_us;
>   };
>   
>   /* XEN_SYSCTL_scheduler_op */

Cheers,

-- 
Julien Grall

Re: [PATCH 7/9] common/libfdt: Use explicitly specified types

[Julien Grall]

Hi Michal,

On 20/06/2022 08:02, Michal Orzel wrote:
> According to MISRA C 2012 Rule 8.1, types shall be explicitly
> specified. Fix all the findings reported by cppcheck with misra addon
> by substituting implicit type 'unsigned' to explicit 'unsigned int'.
> 
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>
> ---
> This patch may not be applicable as these files come from libfdt.

The libfdt code is a verbatim copy of the one shipped in DTC. So any 
changes will have to be first accepted there and then the directory 
re-synced.

Cheers,

-- 
Julien Grall

Re: [PATCH 5/9] include/public: Use explicitly specified types

[Andrew Cooper]

On 20/06/2022 10:54, Julien Grall wrote:
> Hi Michal,
>
> On 20/06/2022 08:02, Michal Orzel wrote:
>> According to MISRA C 2012 Rule 8.1, types shall be explicitly
>> specified. Fix all the findings reported by cppcheck with misra addon
>> by substituting implicit type 'unsigned' to explicit 'unsigned int'.
>>
>> Bump sysctl interface version.
>
> The sysctl version should only be bumped if the ABI has changed.
> AFAICT switching from "unsigned" to "unsigned" will not modify it, so
> I don't think this is necessary.

Yeah.  No need to bump the interface version for this.

~Andrew

Re: [PATCH 5/9] include/public: Use explicitly specified types

[Michal Orzel]

Hi Julien,

On 20.06.2022 11:54, Julien Grall wrote:
> Hi Michal,
> 
> On 20/06/2022 08:02, Michal Orzel wrote:
>> According to MISRA C 2012 Rule 8.1, types shall be explicitly
>> specified. Fix all the findings reported by cppcheck with misra addon
>> by substituting implicit type 'unsigned' to explicit 'unsigned int'.
>>
>> Bump sysctl interface version.
> 
> The sysctl version should only be bumped if the ABI has changed. AFAICT switching from "unsigned" to "unsigned" will not modify it, so I don't think this is necessary.

Sure, I can remove that in v2 but first I'd like to wait at least for xsm patch to be reviewed.
Also as these patches are not dependent from each other, do you think it is worth respinning the reviewed ones?

Cheers,
Michal

Re: [PATCH 5/9] include/public: Use explicitly specified types

[Julien Grall]

On 21/06/2022 09:43, Michal Orzel wrote:
> Hi Julien,

Hi Michal,

> 
> On 20.06.2022 11:54, Julien Grall wrote:
>> Hi Michal,
>>
>> On 20/06/2022 08:02, Michal Orzel wrote:
>>> According to MISRA C 2012 Rule 8.1, types shall be explicitly
>>> specified. Fix all the findings reported by cppcheck with misra addon
>>> by substituting implicit type 'unsigned' to explicit 'unsigned int'.
>>>
>>> Bump sysctl interface version.
>>
>> The sysctl version should only be bumped if the ABI has changed. AFAICT switching from "unsigned" to "unsigned" will not modify it, so I don't think this is necessary.
> 
> Sure, I can remove that in v2 but first I'd like to wait at least for xsm patch to be reviewed.
> Also as these patches are not dependent from each other, do you think it is worth respinning the reviewed ones?

I would suggest to wait until you get input on all the patches before 
respinning.

Cheers,

-- 
Julien Grall

Re: [PATCH 6/9] xsm/flask: Use explicitly specified types

[Jason Andryuk]

On Mon, Jun 20, 2022 at 3:03 AM Michal Orzel <michal.orzel@arm.com> wrote:
>
> According to MISRA C 2012 Rule 8.1, types shall be explicitly
> specified. Fix all the findings reported by cppcheck with misra addon
> by substituting implicit type 'unsigned' to explicit 'unsigned int'.
>
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>

Reviewed-by: Jason Andryuk <jandryuk@gmail.com>

Thanks,
Jason

Re: [PATCH 5/9] include/public: Use explicitly specified types

[Jan Beulich]

On 20.06.2022 09:02, Michal Orzel wrote:
> --- a/xen/include/public/physdev.h
> +++ b/xen/include/public/physdev.h
> @@ -211,8 +211,8 @@ struct physdev_manage_pci_ext {
>      /* IN */
>      uint8_t bus;
>      uint8_t devfn;
> -    unsigned is_extfn;
> -    unsigned is_virtfn;
> +    unsigned int is_extfn;
> +    unsigned int is_virtfn;

It is wrong for us to use unsigned (or unsigned int) here and in sysctl.h.
It should be uint32_t instead, and I think this is a great opportunity to
correct that mistake.

Jan

Re: [PATCH 0/9] MISRA C 2012 8.1 rule fixes

[Jan Beulich]

On 20.06.2022 09:02, Michal Orzel wrote:
> This series fixes all the findings for MISRA C 2012 8.1 rule, reported by
> cppcheck 2.7 with misra addon, for Arm (arm32/arm64 - target allyesconfig).
> Fixing this rule comes down to replacing implicit 'unsigned' with explicit
> 'unsigned int' type as there are no other violations being part of that rule
> in the Xen codebase.

I'm puzzled, I have to admit. While I agree with all the examples in the
doc, I notice that there's no instance of "signed" or "unsigned" there.
Which matches my understanding that "unsigned" and "signed" on their own
(just like "long") are proper types, and hence the omission of "int"
there is not an "omission of an explicit type".

Nevertheless I think we have had the intention to use "unsigned int"
everywhere, but simply for cosmetic / style reasons (while I didn't ever
see anyone request the use of "long int" in place of "long", despite it
also being possible to combine with "double"), so I'm happy to see this
being changed. Just that (for now) I don't buy the justification.

Jan

Re: [PATCH 9/9] drivers/acpi: Use explicitly specified types

[Jan Beulich]

On 20.06.2022 09:02, Michal Orzel wrote:
> According to MISRA C 2012 Rule 8.1, types shall be explicitly
> specified. Fix all the findings reported by cppcheck with misra addon
> by substituting implicit type 'unsigned' to explicit 'unsigned int'.
> 
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>
> ---
> This patch may not be applicable as these files come from Linux.

We've diverged quite far, so the Linux origin isn't really a concern
(anymore).

> --- a/xen/drivers/acpi/tables/tbfadt.c
> +++ b/xen/drivers/acpi/tables/tbfadt.c
> @@ -235,7 +235,7 @@ void __init acpi_tb_create_local_fadt(struct acpi_table_header *table, u32 lengt
>  		ACPI_WARNING((AE_INFO,
>  			      "FADT (revision %u) is longer than ACPI 5.0 version,"
>  			      " truncating length %u to %zu",
> -			      table->revision, (unsigned)length,
> +			      table->revision, (unsigned int)length,

Since we generally try to avoid casts where not needed - did you consider
dropping the cast here instead of fiddling with it? Aiui this wouldn't be
a problem since we assume sizeof(int) >= 4 and since types more narrow
than int would be converted to int (which in turn is generally printing
okay with %u). Strictly speaking it would want to be PRIu32 ...

> --- a/xen/drivers/acpi/tables/tbutils.c
> +++ b/xen/drivers/acpi/tables/tbutils.c
> @@ -481,7 +481,7 @@ acpi_tb_parse_root_table(acpi_physical_address rsdp_address, u8 flags)
>  			if (ACPI_FAILURE(status)) {
>  				ACPI_WARNING((AE_INFO,
>  					      "Truncating %u table entries!",
> -					      (unsigned)
> +					      (unsigned int)
>  					      (acpi_gbl_root_table_list.size -
>  					       acpi_gbl_root_table_list.
>  					       count)));

Same here then, except PRIu32 wouldn't be correct to use in this case.

Jan

Re: [PATCH 5/9] include/public: Use explicitly specified types

[Michal Orzel]

On 22.06.2022 12:16, Jan Beulich wrote:
> On 20.06.2022 09:02, Michal Orzel wrote:
>> --- a/xen/include/public/physdev.h
>> +++ b/xen/include/public/physdev.h
>> @@ -211,8 +211,8 @@ struct physdev_manage_pci_ext {
>>      /* IN */
>>      uint8_t bus;
>>      uint8_t devfn;
>> -    unsigned is_extfn;
>> -    unsigned is_virtfn;
>> +    unsigned int is_extfn;
>> +    unsigned int is_virtfn;
> 
> It is wrong for us to use unsigned (or unsigned int) here and in sysctl.h.
> It should be uint32_t instead, and I think this is a great opportunity to
> correct that mistake.
> 
That is perfectly fine for me to do.

> Jan

Cheers,
Michal

Re: [PATCH 9/9] drivers/acpi: Use explicitly specified types

[Michal Orzel]

On 22.06.2022 12:36, Jan Beulich wrote:
> On 20.06.2022 09:02, Michal Orzel wrote:
>> According to MISRA C 2012 Rule 8.1, types shall be explicitly
>> specified. Fix all the findings reported by cppcheck with misra addon
>> by substituting implicit type 'unsigned' to explicit 'unsigned int'.
>>
>> Signed-off-by: Michal Orzel <michal.orzel@arm.com>
>> ---
>> This patch may not be applicable as these files come from Linux.
> 
> We've diverged quite far, so the Linux origin isn't really a concern
> (anymore).
> 
Ok.
>> --- a/xen/drivers/acpi/tables/tbfadt.c
>> +++ b/xen/drivers/acpi/tables/tbfadt.c
>> @@ -235,7 +235,7 @@ void __init acpi_tb_create_local_fadt(struct acpi_table_header *table, u32 lengt
>>  		ACPI_WARNING((AE_INFO,
>>  			      "FADT (revision %u) is longer than ACPI 5.0 version,"
>>  			      " truncating length %u to %zu",
>> -			      table->revision, (unsigned)length,
>> +			      table->revision, (unsigned int)length,
> 
> Since we generally try to avoid casts where not needed - did you consider
> dropping the cast here instead of fiddling with it? Aiui this wouldn't be
> a problem since we assume sizeof(int) >= 4 and since types more narrow
> than int would be converted to int (which in turn is generally printing
> okay with %u). Strictly speaking it would want to be PRIu32 ...
> 
The reason I did not consider it was to make the review process easier by performing only
the mechanical change being part of fixing 8.1 rule. However I'm fully ok to drop the cast.

Changing the format specifier to PRIu32 for length would require using PRIu8 for table->revision
to be coherent.

>> --- a/xen/drivers/acpi/tables/tbutils.c
>> +++ b/xen/drivers/acpi/tables/tbutils.c
>> @@ -481,7 +481,7 @@ acpi_tb_parse_root_table(acpi_physical_address rsdp_address, u8 flags)
>>  			if (ACPI_FAILURE(status)) {
>>  				ACPI_WARNING((AE_INFO,
>>  					      "Truncating %u table entries!",
>> -					      (unsigned)
>> +					      (unsigned int)
>>  					      (acpi_gbl_root_table_list.size -
>>  					       acpi_gbl_root_table_list.
>>  					       count)));
> 
> Same here then, except PRIu32 wouldn't be correct to use in this case.
> 
Why is it so given that both size and count are of type u32?

> Jan

Cheers,
Michal

Re: [PATCH 9/9] drivers/acpi: Use explicitly specified types

[Jan Beulich]

On 22.06.2022 13:09, Michal Orzel wrote:
> On 22.06.2022 12:36, Jan Beulich wrote:
>> On 20.06.2022 09:02, Michal Orzel wrote:
>>> --- a/xen/drivers/acpi/tables/tbutils.c
>>> +++ b/xen/drivers/acpi/tables/tbutils.c
>>> @@ -481,7 +481,7 @@ acpi_tb_parse_root_table(acpi_physical_address rsdp_address, u8 flags)
>>>  			if (ACPI_FAILURE(status)) {
>>>  				ACPI_WARNING((AE_INFO,
>>>  					      "Truncating %u table entries!",
>>> -					      (unsigned)
>>> +					      (unsigned int)
>>>  					      (acpi_gbl_root_table_list.size -
>>>  					       acpi_gbl_root_table_list.
>>>  					       count)));
>>
>> Same here then, except PRIu32 wouldn't be correct to use in this case.
>>
> Why is it so given that both size and count are of type u32?

Because the promoted type (i.e. the type of the result of the subtraction)
isn't uint32_t (and will never be). It'll be "unsigned int" when
sizeof(int) == 4 (and in this case it'll happen to alias uint32_t) and
just "int" when sizeof(int) > 4 (not even aliasing int32_t).

Jan

Re: [PATCH 0/9] MISRA C 2012 8.1 rule fixes

[Michal Orzel]

Hi Jan,

On 22.06.2022 12:25, Jan Beulich wrote:
> On 20.06.2022 09:02, Michal Orzel wrote:
>> This series fixes all the findings for MISRA C 2012 8.1 rule, reported by
>> cppcheck 2.7 with misra addon, for Arm (arm32/arm64 - target allyesconfig).
>> Fixing this rule comes down to replacing implicit 'unsigned' with explicit
>> 'unsigned int' type as there are no other violations being part of that rule
>> in the Xen codebase.
> 
> I'm puzzled, I have to admit. While I agree with all the examples in the
> doc, I notice that there's no instance of "signed" or "unsigned" there.
> Which matches my understanding that "unsigned" and "signed" on their own
> (just like "long") are proper types, and hence the omission of "int"
> there is not an "omission of an explicit type".
> 
Cppcheck was choosed as a tool for MISRA checking and it is considering it as a violation.
It treats unsigned as an implicit type. You can see this flag in cppcheck source code:

"fIsImplicitInt          = (1U << 31),   // Is "int" token implicitly added?"

> Nevertheless I think we have had the intention to use "unsigned int"
> everywhere, but simply for cosmetic / style reasons (while I didn't ever
> see anyone request the use of "long int" in place of "long", despite it
> also being possible to combine with "double"), so I'm happy to see this
> being changed. Just that (for now) I don't buy the justification.
> 
> Jan

Cheers,
Michal

Re: [PATCH 0/9] MISRA C 2012 8.1 rule fixes

[Jan Beulich]

On 22.06.2022 14:55, Michal Orzel wrote:
> On 22.06.2022 12:25, Jan Beulich wrote:
>> On 20.06.2022 09:02, Michal Orzel wrote:
>>> This series fixes all the findings for MISRA C 2012 8.1 rule, reported by
>>> cppcheck 2.7 with misra addon, for Arm (arm32/arm64 - target allyesconfig).
>>> Fixing this rule comes down to replacing implicit 'unsigned' with explicit
>>> 'unsigned int' type as there are no other violations being part of that rule
>>> in the Xen codebase.
>>
>> I'm puzzled, I have to admit. While I agree with all the examples in the
>> doc, I notice that there's no instance of "signed" or "unsigned" there.
>> Which matches my understanding that "unsigned" and "signed" on their own
>> (just like "long") are proper types, and hence the omission of "int"
>> there is not an "omission of an explicit type".
>>
> Cppcheck was choosed as a tool for MISRA checking and it is considering it as a violation.

Which by no means indicates that the tool pointing out something as a
violation actually is one.

> It treats unsigned as an implicit type. You can see this flag in cppcheck source code:
> 
> "fIsImplicitInt          = (1U << 31),   // Is "int" token implicitly added?"

Neither the name of the variable nor the comment clarify that this is about
the specific case of "unsigned". As said there's also the fact that they
don't appear to point out the lack of "int" when seeing plain "long" (or
"long long"). I fully agree that "extern x;" or "const y;" lack explicit
"int".

Jan

Re: [PATCH 0/9] MISRA C 2012 8.1 rule fixes

[Bertrand Marquis]

Hi Jan,

> On 22 Jun 2022, at 14:01, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 22.06.2022 14:55, Michal Orzel wrote:
>> On 22.06.2022 12:25, Jan Beulich wrote:
>>> On 20.06.2022 09:02, Michal Orzel wrote:
>>>> This series fixes all the findings for MISRA C 2012 8.1 rule, reported by
>>>> cppcheck 2.7 with misra addon, for Arm (arm32/arm64 - target allyesconfig).
>>>> Fixing this rule comes down to replacing implicit 'unsigned' with explicit
>>>> 'unsigned int' type as there are no other violations being part of that rule
>>>> in the Xen codebase.
>>> 
>>> I'm puzzled, I have to admit. While I agree with all the examples in the
>>> doc, I notice that there's no instance of "signed" or "unsigned" there.
>>> Which matches my understanding that "unsigned" and "signed" on their own
>>> (just like "long") are proper types, and hence the omission of "int"
>>> there is not an "omission of an explicit type".
>>> 
>> Cppcheck was choosed as a tool for MISRA checking and it is considering it as a violation.
> 
> Which by no means indicates that the tool pointing out something as a
> violation actually is one.
> 
>> It treats unsigned as an implicit type. You can see this flag in cppcheck source code:
>> 
>> "fIsImplicitInt          = (1U << 31),   // Is "int" token implicitly added?"
> 
> Neither the name of the variable nor the comment clarify that this is about
> the specific case of "unsigned". As said there's also the fact that they
> don't appear to point out the lack of "int" when seeing plain "long" (or
> "long long"). I fully agree that "extern x;" or "const y;" lack explicit
> "int".

I am a bit puzzled here trying to understand what you actually want here.

Do you suggest the change is ok but you are not ok with the fact that is flagged
as MISRA fix even though cppcheck is saying otherwise ?

Bertrand

Re: [PATCH 0/9] MISRA C 2012 8.1 rule fixes

[Jan Beulich]

On 22.06.2022 15:55, Bertrand Marquis wrote:
> Hi Jan,
> 
>> On 22 Jun 2022, at 14:01, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 22.06.2022 14:55, Michal Orzel wrote:
>>> On 22.06.2022 12:25, Jan Beulich wrote:
>>>> On 20.06.2022 09:02, Michal Orzel wrote:
>>>>> This series fixes all the findings for MISRA C 2012 8.1 rule, reported by
>>>>> cppcheck 2.7 with misra addon, for Arm (arm32/arm64 - target allyesconfig).
>>>>> Fixing this rule comes down to replacing implicit 'unsigned' with explicit
>>>>> 'unsigned int' type as there are no other violations being part of that rule
>>>>> in the Xen codebase.
>>>>
>>>> I'm puzzled, I have to admit. While I agree with all the examples in the
>>>> doc, I notice that there's no instance of "signed" or "unsigned" there.
>>>> Which matches my understanding that "unsigned" and "signed" on their own
>>>> (just like "long") are proper types, and hence the omission of "int"
>>>> there is not an "omission of an explicit type".
>>>>
>>> Cppcheck was choosed as a tool for MISRA checking and it is considering it as a violation.
>>
>> Which by no means indicates that the tool pointing out something as a
>> violation actually is one.
>>
>>> It treats unsigned as an implicit type. You can see this flag in cppcheck source code:
>>>
>>> "fIsImplicitInt          = (1U << 31),   // Is "int" token implicitly added?"
>>
>> Neither the name of the variable nor the comment clarify that this is about
>> the specific case of "unsigned". As said there's also the fact that they
>> don't appear to point out the lack of "int" when seeing plain "long" (or
>> "long long"). I fully agree that "extern x;" or "const y;" lack explicit
>> "int".
> 
> I am a bit puzzled here trying to understand what you actually want here.
> 
> Do you suggest the change is ok but you are not ok with the fact that is flagged
> as MISRA fix even though cppcheck is saying otherwise ?

First of all I'd like to understand whether what we're talking about here
are actually violations (and if so, why that is). Further actions / requests
depend on the answer.

Jan

Re: [PATCH 0/9] MISRA C 2012 8.1 rule fixes

[Bertrand Marquis]

Hi,

> On 22 Jun 2022, at 15:10, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 22.06.2022 15:55, Bertrand Marquis wrote:
>> Hi Jan,
>> 
>>> On 22 Jun 2022, at 14:01, Jan Beulich <jbeulich@suse.com> wrote:
>>> 
>>> On 22.06.2022 14:55, Michal Orzel wrote:
>>>> On 22.06.2022 12:25, Jan Beulich wrote:
>>>>> On 20.06.2022 09:02, Michal Orzel wrote:
>>>>>> This series fixes all the findings for MISRA C 2012 8.1 rule, reported by
>>>>>> cppcheck 2.7 with misra addon, for Arm (arm32/arm64 - target allyesconfig).
>>>>>> Fixing this rule comes down to replacing implicit 'unsigned' with explicit
>>>>>> 'unsigned int' type as there are no other violations being part of that rule
>>>>>> in the Xen codebase.
>>>>> 
>>>>> I'm puzzled, I have to admit. While I agree with all the examples in the
>>>>> doc, I notice that there's no instance of "signed" or "unsigned" there.
>>>>> Which matches my understanding that "unsigned" and "signed" on their own
>>>>> (just like "long") are proper types, and hence the omission of "int"
>>>>> there is not an "omission of an explicit type".
>>>>> 
>>>> Cppcheck was choosed as a tool for MISRA checking and it is considering it as a violation.
>>> 
>>> Which by no means indicates that the tool pointing out something as a
>>> violation actually is one.
>>> 
>>>> It treats unsigned as an implicit type. You can see this flag in cppcheck source code:
>>>> 
>>>> "fIsImplicitInt = (1U << 31), // Is "int" token implicitly added?"
>>> 
>>> Neither the name of the variable nor the comment clarify that this is about
>>> the specific case of "unsigned". As said there's also the fact that they
>>> don't appear to point out the lack of "int" when seeing plain "long" (or
>>> "long long"). I fully agree that "extern x;" or "const y;" lack explicit
>>> "int".
>> 
>> I am a bit puzzled here trying to understand what you actually want here.
>> 
>> Do you suggest the change is ok but you are not ok with the fact that is flagged
>> as MISRA fix even though cppcheck is saying otherwise ?
> 
> First of all I'd like to understand whether what we're talking about here
> are actually violations (and if so, why that is). Further actions / requests
> depend on the answer.

I would say yes but this would need to be confirmed by Roberto I guess.
In any case if we think this is something we want and the change is right
and cppcheck is saying that it solves a violation, I am wondering what is
the point of discussing that extensively this change.

Cheers
Bertrand

Re: [PATCH 0/9] MISRA C 2012 8.1 rule fixes

[Jan Beulich]

On 22.06.2022 16:27, Bertrand Marquis wrote:
> Hi,
> 
>> On 22 Jun 2022, at 15:10, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 22.06.2022 15:55, Bertrand Marquis wrote:
>>> Hi Jan,
>>>
>>>> On 22 Jun 2022, at 14:01, Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 22.06.2022 14:55, Michal Orzel wrote:
>>>>> On 22.06.2022 12:25, Jan Beulich wrote:
>>>>>> On 20.06.2022 09:02, Michal Orzel wrote:
>>>>>>> This series fixes all the findings for MISRA C 2012 8.1 rule, reported by
>>>>>>> cppcheck 2.7 with misra addon, for Arm (arm32/arm64 - target allyesconfig).
>>>>>>> Fixing this rule comes down to replacing implicit 'unsigned' with explicit
>>>>>>> 'unsigned int' type as there are no other violations being part of that rule
>>>>>>> in the Xen codebase.
>>>>>>
>>>>>> I'm puzzled, I have to admit. While I agree with all the examples in the
>>>>>> doc, I notice that there's no instance of "signed" or "unsigned" there.
>>>>>> Which matches my understanding that "unsigned" and "signed" on their own
>>>>>> (just like "long") are proper types, and hence the omission of "int"
>>>>>> there is not an "omission of an explicit type".
>>>>>>
>>>>> Cppcheck was choosed as a tool for MISRA checking and it is considering it as a violation.
>>>>
>>>> Which by no means indicates that the tool pointing out something as a
>>>> violation actually is one.
>>>>
>>>>> It treats unsigned as an implicit type. You can see this flag in cppcheck source code:
>>>>>
>>>>> "fIsImplicitInt = (1U << 31), // Is "int" token implicitly added?"
>>>>
>>>> Neither the name of the variable nor the comment clarify that this is about
>>>> the specific case of "unsigned". As said there's also the fact that they
>>>> don't appear to point out the lack of "int" when seeing plain "long" (or
>>>> "long long"). I fully agree that "extern x;" or "const y;" lack explicit
>>>> "int".
>>>
>>> I am a bit puzzled here trying to understand what you actually want here.
>>>
>>> Do you suggest the change is ok but you are not ok with the fact that is flagged
>>> as MISRA fix even though cppcheck is saying otherwise ?
>>
>> First of all I'd like to understand whether what we're talking about here
>> are actually violations (and if so, why that is). Further actions / requests
>> depend on the answer.
> 
> I would say yes but this would need to be confirmed by Roberto I guess.
> In any case if we think this is something we want and the change is right
> and cppcheck is saying that it solves a violation, I am wondering what is
> the point of discussing that extensively this change.

Because imo a patch shouldn't be committed with a misleading description,
if at all possible. Even less so several such patches in one go.

Jan

Re: [PATCH 0/9] MISRA C 2012 8.1 rule fixes

[Stefano Stabellini]

+Roberto


Hi Roberto,

A quick question about Rule 8.1.


Michal sent a patch series to fix Xen against Rule 8.1 (here is a link
if you are interested: https://marc.info/?l=xen-devel&m=165570851227125)

Although we all generally agree that the changes are a good thing, there
was a question about the rule itself. Specifically, is the following
actually a violation?

  unsigned x;


Looking through the examples in the MISRA document I can see various
instances of more confusing and obvious violations such as:

  const x;
  extern x;

but no examples of using "unsigned" without "int". Do you know if it is
considered a violation?


Thanks!

Cheers,

Stefano



On Wed, 22 Jun 2022, Jan Beulich wrote:
> >>>>> On 22.06.2022 12:25, Jan Beulich wrote:
> >>>>>> On 20.06.2022 09:02, Michal Orzel wrote:
> >>>>>>> This series fixes all the findings for MISRA C 2012 8.1 rule, reported by
> >>>>>>> cppcheck 2.7 with misra addon, for Arm (arm32/arm64 - target allyesconfig).
> >>>>>>> Fixing this rule comes down to replacing implicit 'unsigned' with explicit
> >>>>>>> 'unsigned int' type as there are no other violations being part of that rule
> >>>>>>> in the Xen codebase.
> >>>>>>
> >>>>>> I'm puzzled, I have to admit. While I agree with all the examples in the
> >>>>>> doc, I notice that there's no instance of "signed" or "unsigned" there.
> >>>>>> Which matches my understanding that "unsigned" and "signed" on their own
> >>>>>> (just like "long") are proper types, and hence the omission of "int"
> >>>>>> there is not an "omission of an explicit type".

[...]

> >>>> Neither the name of the variable nor the comment clarify that this is about
> >>>> the specific case of "unsigned". As said there's also the fact that they
> >>>> don't appear to point out the lack of "int" when seeing plain "long" (or
> >>>> "long long"). I fully agree that "extern x;" or "const y;" lack explicit
> >>>> "int".

Re: [PATCH 0/9] MISRA C 2012 8.1 rule fixes

[Jan Beulich]

On 22.06.2022 21:23, Stefano Stabellini wrote:
> A quick question about Rule 8.1.
> 
> 
> Michal sent a patch series to fix Xen against Rule 8.1 (here is a link
> if you are interested: https://marc.info/?l=xen-devel&m=165570851227125)
> 
> Although we all generally agree that the changes are a good thing, there
> was a question about the rule itself. Specifically, is the following
> actually a violation?
> 
>   unsigned x;
> 
> 
> Looking through the examples in the MISRA document I can see various
> instances of more confusing and obvious violations such as:
> 
>   const x;
>   extern x;
> 
> but no examples of using "unsigned" without "int". Do you know if it is
> considered a violation?

And if it is, by implication would plain "long" also be a violation?

Jan

Re: [PATCH 0/9] MISRA C 2012 8.1 rule fixes

[Roberto Bagnara]

Hi there.

Rule 8.1 only applies to C90 code, as all the violating instances are
syntax errors in C99 and later versions of the language.  So,
the following line does not contain a violation of Rule 8.1:

     unsigned x;

It does contain a violation of Directive 4.6, though, whose correct
handling depends on the intention (uint32_t, uin64_t, size_t, ...).

As a side note (still on the theme of the many ways of referring
to a concrete type) Rule 6.1 says not to use plain int for a bitfield
and using an explicitly signed or unsigned type instead.
(Note that Directive 4.6 does not apply to bitfield types.)
So

     int field1:2;

is not compliant; the following are compliant:

     signed int   field1:2;
     unsigned int field2:3;

But also the following are compliant, and we much favor
this variant as the specification of "int" buys nothing
and can even mislead someone into thinking that more bits
are reserved:

     signed   field1:2;
     unsigned field2:3;

I mention this to encourage, as a matter of style, not to add
"int" on bitfield types currently specified as "signed" or "unsigned".
Kind regards,

    Roberto

On 22/06/22 21:23, Stefano Stabellini wrote:
> +Roberto
> 
> 
> Hi Roberto,
> 
> A quick question about Rule 8.1.
> 
> 
> Michal sent a patch series to fix Xen against Rule 8.1 (here is a link
> if you are interested: https://marc.info/?l=xen-devel&m=165570851227125)
> 
> Although we all generally agree that the changes are a good thing, there
> was a question about the rule itself. Specifically, is the following
> actually a violation?
> 
>    unsigned x;
> 
> 
> Looking through the examples in the MISRA document I can see various
> instances of more confusing and obvious violations such as:
> 
>    const x;
>    extern x;
> 
> but no examples of using "unsigned" without "int". Do you know if it is
> considered a violation?
> 
> 
> Thanks!
> 
> Cheers,
> 
> Stefano
> 
> 
> 
> On Wed, 22 Jun 2022, Jan Beulich wrote:
>>>>>>> On 22.06.2022 12:25, Jan Beulich wrote:
>>>>>>>> On 20.06.2022 09:02, Michal Orzel wrote:
>>>>>>>>> This series fixes all the findings for MISRA C 2012 8.1 rule, reported by
>>>>>>>>> cppcheck 2.7 with misra addon, for Arm (arm32/arm64 - target allyesconfig).
>>>>>>>>> Fixing this rule comes down to replacing implicit 'unsigned' with explicit
>>>>>>>>> 'unsigned int' type as there are no other violations being part of that rule
>>>>>>>>> in the Xen codebase.
>>>>>>>>
>>>>>>>> I'm puzzled, I have to admit. While I agree with all the examples in the
>>>>>>>> doc, I notice that there's no instance of "signed" or "unsigned" there.
>>>>>>>> Which matches my understanding that "unsigned" and "signed" on their own
>>>>>>>> (just like "long") are proper types, and hence the omission of "int"
>>>>>>>> there is not an "omission of an explicit type".
> 
> [...]
> 
>>>>>> Neither the name of the variable nor the comment clarify that this is about
>>>>>> the specific case of "unsigned". As said there's also the fact that they
>>>>>> don't appear to point out the lack of "int" when seeing plain "long" (or
>>>>>> "long long"). I fully agree that "extern x;" or "const y;" lack explicit
>>>>>> "int".

Re: [PATCH 0/9] MISRA C 2012 8.1 rule fixes

[Jan Beulich]

On 23.06.2022 09:37, Roberto Bagnara wrote:
> Rule 8.1 only applies to C90 code, as all the violating instances are
> syntax errors in C99 and later versions of the language.  So,
> the following line does not contain a violation of Rule 8.1:
> 
>      unsigned x;
> 
> It does contain a violation of Directive 4.6, though, whose correct
> handling depends on the intention (uint32_t, uin64_t, size_t, ...).

Interesting - this goes straight against a rule we have set in
./CODING_STYLE. I'm also puzzled by you including size_t in your list
of examples, when the spec doesn't. The sole "goal" of the directive
(which is advisory only anyway) is to be able to determine allocation
size. size_t size, however, varies as much as short, int, long, etc
do.

Jan

Re: [PATCH 0/9] MISRA C 2012 8.1 rule fixes

[Stefano Stabellini]

On Thu, 23 Jun 2022, Jan Beulich wrote:
> On 23.06.2022 09:37, Roberto Bagnara wrote:
> > Rule 8.1 only applies to C90 code, as all the violating instances are
> > syntax errors in C99 and later versions of the language.  So,
> > the following line does not contain a violation of Rule 8.1:
> > 
> >      unsigned x;
> > 
> > It does contain a violation of Directive 4.6, though, whose correct
> > handling depends on the intention (uint32_t, uin64_t, size_t, ...).

Hi Roberto,

Thank you very much for the quick reply and very clear answer!


> Interesting - this goes straight against a rule we have set in
> ./CODING_STYLE. I'm also puzzled by you including size_t in your list
> of examples, when the spec doesn't. The sole "goal" of the directive
> (which is advisory only anyway) is to be able to determine allocation
> size. size_t size, however, varies as much as short, int, long, etc
> do.

I wouldn't worry about Directive 4.6 for now. We'll talk about it when
we get to it. (Also we already require uint32_t, uint64_t, etc. in all
external interfaces and ABIs which I think is what Dir 4.6 cares about
the most.)

For this series, I suggest to keep the patches because "unsigned int" is
better than "unsigned" from a style perspective, but we need to rephrase
the commit messages because we cannot claim they are fixing Rule 8.1.
Also, thank Jan for spotting the misunderstanding!

Re: [PATCH 0/9] MISRA C 2012 8.1 rule fixes

[Roberto Bagnara]

Hi Jan.

I know I will sound pedantic ;-)  but an important fact about
the MISRA standards is that reading the headline alone is almost
never enough.  In the specific of (advisory) Directive 4.6,
the Rationale says, among other things:

     It might be desirable not to apply this guideline when
     interfacing with The Standard Library or code outside
     the project’s control.

For this reason, size_t is typically set as an exception in the
tool configuration.  To properly deal with the many Standard Library
functions returning int, one can use a typedef named something
like "lib_int_t" to write, e.g.,

   const lib_int_t r = strncmp(...);

The lib_int_t typedef can be used with a suitable tool configuration,
just as I mentioned one would do with size_t.
Kind regards,

    Roberto

On 23/06/22 09:51, Jan Beulich wrote:
> On 23.06.2022 09:37, Roberto Bagnara wrote:
>> Rule 8.1 only applies to C90 code, as all the violating instances are
>> syntax errors in C99 and later versions of the language.  So,
>> the following line does not contain a violation of Rule 8.1:
>>
>>       unsigned x;
>>
>> It does contain a violation of Directive 4.6, though, whose correct
>> handling depends on the intention (uint32_t, uin64_t, size_t, ...).
> 
> Interesting - this goes straight against a rule we have set in
> ./CODING_STYLE. I'm also puzzled by you including size_t in your list
> of examples, when the spec doesn't. The sole "goal" of the directive
> (which is advisory only anyway) is to be able to determine allocation
> size. size_t size, however, varies as much as short, int, long, etc
> do.
> 
> Jan