[Qemu-devel] [PULL 00/22] misc queue

[Qemu-devel] [PULL 00/22] misc queue

[Peter Maydell]

This is a colletion of miscellaneous patches (mostly mine),
which fix minor bugs or do some refactoring/cleanup.
No user-visible changes in here.

thanks
-- PMM

The following changes since commit 0f98c9945899c5dfacd5a410ff04178eda605a16:

  Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2018-12-12' into staging (2018-12-14 10:19:47 +0000)

are available in the Git repository at:

  https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-misc-20181214

for you to fetch changes up to bbac02f1e8edfd0663543f6fdad1e7094d860b29:

  virt: Fix broken indentation (2018-12-14 13:30:55 +0000)

----------------------------------------------------------------
miscellaneous patches:
 * checkpatch.pl: Enforce multiline comment syntax
 * Rename cpu_physical_memory_write_rom() to address_space_write_rom()
 * disas, monitor, elf_ops: Use address_space_read() to read memory
 * Remove load_image() in favour of load_image_size()
 * Fix some minor memory leaks in arm boards/devices
 * virt: fix broken indentation

----------------------------------------------------------------
Eduardo Habkost (1):
      virt: Fix broken indentation

Peter Maydell (21):
      scripts/checkpatch.pl: Enforce multiline comment syntax
      exec.c: Rename cpu_physical_memory_write_rom_internal()
      Rename cpu_physical_memory_write_rom() to address_space_write_rom()
      disas.c: Use address_space_read() to read memory
      monitor: Use address_space_read() to read memory
      elf_ops.h: Use address_space_write() to write memory
      hw/ppc/mac_newworld, mac_oldworld: Don't use load_image()
      hw/ppc/ppc405_boards: Don't use load_image()
      hw/smbios/smbios.c: Don't use load_image()
      hw/pci/pci.c: Don't use load_image()
      hw/i386/pc.c: Don't use load_image()
      hw/i386/multiboot.c: Don't use load_image()
      hw/block/tc58128.c: Don't use load_image()
      device_tree.c: Don't use load_image()
      hw/core/loader.c: Remove load_image()
      include/hw/loader.h: Document load_image_size()
      target/arm: Free name string in ARMCPRegInfo hashtable entries
      hw/arm/mps2-tz.c: Free mscname string in make_dma()
      hw/sd/sdhci: Don't leak memory region in sdhci_sysbus_realize()
      tests/test-arm-mptimer: Don't leak string memory
      target/arm: Create timers in realize, not init

 include/exec/cpu-common.h   |   2 -
 include/exec/memory.h       |  26 ++++++++
 include/hw/elf_ops.h        |   4 +-
 include/hw/loader.h         |  17 ++++-
 device_tree.c               |   2 +-
 disas.c                     |   5 +-
 exec.c                      |  30 +++++----
 hw/arm/mps2-tz.c            |   1 +
 hw/arm/virt.c               |   2 +-
 hw/block/tc58128.c          |   3 +-
 hw/core/loader.c            |  29 +--------
 hw/i386/multiboot.c         |   6 +-
 hw/i386/pc.c                |  22 ++++---
 hw/intc/apic.c              |   7 +-
 hw/misc/tz-mpc.c            |   2 +-
 hw/pci/pci.c                |   6 +-
 hw/ppc/mac_newworld.c       |  10 ++-
 hw/ppc/mac_oldworld.c       |  10 ++-
 hw/ppc/ppc405_boards.c      |  12 ++--
 hw/sd/sdhci.c               |   5 +-
 hw/smbios/smbios.c          |   2 +-
 hw/sparc/sun4m.c            |   5 +-
 monitor.c                   |   8 ++-
 target/arm/cpu.c            |  33 +++++++---
 tests/test-arm-mptimer.c    | 153 ++++++++++++++++++++------------------------
 docs/devel/loads-stores.rst |  35 +++++-----
 scripts/checkpatch.pl       |  48 ++++++++++++++
 27 files changed, 287 insertions(+), 198 deletions(-)

[Qemu-devel] [PULL 01/22] scripts/checkpatch.pl: Enforce multiline comment syntax

[Peter Maydell]

We now require Linux-kernel-style multiline comments:
    /*
     * line one
     * line two
     */

Enforce this in checkpatch.pl, by backporting the relevant
parts of the Linux kernel's checkpatch.pl. (The only changes
needed are that Linux's checkpatch.pl WARN() function takes
an extra argument that ours does not, and the kernel has a
special case for networking code we don't want.)"

The kernel's checkpatch does not enforce "leading /* on
a line of its own, so that part is unique to QEMU's checkpatch.

Sample warning output:

WARNING: Block comments use a leading /* on a separate line
#34: FILE: hw/intc/arm_gicv3_common.c:39:
+    /* Older versions of QEMU had a bug in the handling of state save/restore

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Acked-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/checkpatch.pl | 48 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a892a6cc7c3..18e16b79dfc 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1569,6 +1569,54 @@ sub process {
 # check we are in a valid C source file if not then ignore this hunk
 		next if ($realfile !~ /\.(h|c|cpp)$/);
 
+# Block comment styles
+
+		# Block comments use /* on a line of its own
+		if ($rawline !~ m@^\+.*/\*.*\*/[ \t]*$@ &&	#inline /*...*/
+		    $rawline =~ m@^\+.*/\*\*?[ \t]*.+[ \t]*$@) { # /* or /** non-blank
+			WARN("Block comments use a leading /* on a separate line\n" . $herecurr);
+		}
+
+# Block comments use * on subsequent lines
+		if ($prevline =~ /$;[ \t]*$/ &&			#ends in comment
+		    $prevrawline =~ /^\+.*?\/\*/ &&		#starting /*
+		    $prevrawline !~ /\*\/[ \t]*$/ &&		#no trailing */
+		    $rawline =~ /^\+/ &&			#line is new
+		    $rawline !~ /^\+[ \t]*\*/) {		#no leading *
+			WARN("Block comments use * on subsequent lines\n" . $hereprev);
+		}
+
+# Block comments use */ on trailing lines
+		if ($rawline !~ m@^\+[ \t]*\*/[ \t]*$@ &&	#trailing */
+		    $rawline !~ m@^\+.*/\*.*\*/[ \t]*$@ &&	#inline /*...*/
+		    $rawline !~ m@^\+.*\*{2,}/[ \t]*$@ &&	#trailing **/
+		    $rawline =~ m@^\+[ \t]*.+\*\/[ \t]*$@) {	#non blank */
+			WARN("Block comments use a trailing */ on a separate line\n" . $herecurr);
+		}
+
+# Block comment * alignment
+		if ($prevline =~ /$;[ \t]*$/ &&			#ends in comment
+		    $line =~ /^\+[ \t]*$;/ &&			#leading comment
+		    $rawline =~ /^\+[ \t]*\*/ &&		#leading *
+		    (($prevrawline =~ /^\+.*?\/\*/ &&		#leading /*
+		      $prevrawline !~ /\*\/[ \t]*$/) ||		#no trailing */
+		     $prevrawline =~ /^\+[ \t]*\*/)) {		#leading *
+			my $oldindent;
+			$prevrawline =~ m@^\+([ \t]*/?)\*@;
+			if (defined($1)) {
+				$oldindent = expand_tabs($1);
+			} else {
+				$prevrawline =~ m@^\+(.*/?)\*@;
+				$oldindent = expand_tabs($1);
+			}
+			$rawline =~ m@^\+([ \t]*)\*@;
+			my $newindent = $1;
+			$newindent = expand_tabs($newindent);
+			if (length($oldindent) ne length($newindent)) {
+				WARN("Block comments should align the * on each line\n" . $hereprev);
+			}
+		}
+
 # Check for potential 'bare' types
 		my ($stat, $cond, $line_nr_next, $remain_next, $off_next,
 		    $realline_next);
-- 
2.19.2

[Qemu-devel] [PULL 02/22] exec.c: Rename cpu_physical_memory_write_rom_internal()

[Peter Maydell]

Rename cpu_physical_memory_write_rom_internal() to
address_space_write_rom_internal(), and make it take
MemTxAttrs and return a MemTxResult. This brings its
API into line with address_space_write().

This is an internal function to exec.c; fixing its API
will allow us to change the global function
cpu_physical_memory_write_rom().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Message-id: 20181122133507.30950-2-peter.maydell@linaro.org
---
 exec.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/exec.c b/exec.c
index bb6170dbffe..92679508ba3 100644
--- a/exec.c
+++ b/exec.c
@@ -3388,8 +3388,12 @@ enum write_rom_type {
     FLUSH_CACHE,
 };
 
-static inline void cpu_physical_memory_write_rom_internal(AddressSpace *as,
-    hwaddr addr, const uint8_t *buf, int len, enum write_rom_type type)
+static inline MemTxResult address_space_write_rom_internal(AddressSpace *as,
+                                                           hwaddr addr,
+                                                           MemTxAttrs attrs,
+                                                           const uint8_t *buf,
+                                                           int len,
+                                                           enum write_rom_type type)
 {
     hwaddr l;
     uint8_t *ptr;
@@ -3399,8 +3403,7 @@ static inline void cpu_physical_memory_write_rom_internal(AddressSpace *as,
     rcu_read_lock();
     while (len > 0) {
         l = len;
-        mr = address_space_translate(as, addr, &addr1, &l, true,
-                                     MEMTXATTRS_UNSPECIFIED);
+        mr = address_space_translate(as, addr, &addr1, &l, true, attrs);
 
         if (!(memory_region_is_ram(mr) ||
               memory_region_is_romd(mr))) {
@@ -3423,13 +3426,15 @@ static inline void cpu_physical_memory_write_rom_internal(AddressSpace *as,
         addr += l;
     }
     rcu_read_unlock();
+    return MEMTX_OK;
 }
 
 /* used for ROM loading : can write in RAM and ROM */
 void cpu_physical_memory_write_rom(AddressSpace *as, hwaddr addr,
                                    const uint8_t *buf, int len)
 {
-    cpu_physical_memory_write_rom_internal(as, addr, buf, len, WRITE_DATA);
+    address_space_write_rom_internal(as, addr, MEMTXATTRS_UNSPECIFIED,
+                                     buf, len, WRITE_DATA);
 }
 
 void cpu_flush_icache_range(hwaddr start, int len)
@@ -3444,8 +3449,9 @@ void cpu_flush_icache_range(hwaddr start, int len)
         return;
     }
 
-    cpu_physical_memory_write_rom_internal(&address_space_memory,
-                                           start, NULL, len, FLUSH_CACHE);
+    address_space_write_rom_internal(&address_space_memory,
+                                     start, MEMTXATTRS_UNSPECIFIED,
+                                     NULL, len, FLUSH_CACHE);
 }
 
 typedef struct {
-- 
2.19.2

[Qemu-devel] [PULL 03/22] Rename cpu_physical_memory_write_rom() to address_space_write_rom()

[Peter Maydell]

The API of cpu_physical_memory_write_rom() is odd, because it
takes an AddressSpace, unlike all the other cpu_physical_memory_*
access functions. Rename it to address_space_write_rom(), and
bring its API into line with address_space_write().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Message-id: 20181122133507.30950-3-peter.maydell@linaro.org
---
 include/exec/cpu-common.h   |  2 --
 include/exec/memory.h       | 26 ++++++++++++++++++++++++++
 exec.c                      | 14 ++++++++------
 hw/core/loader.c            |  4 ++--
 hw/intc/apic.c              |  7 ++++---
 hw/misc/tz-mpc.c            |  2 +-
 hw/sparc/sun4m.c            |  5 +++--
 docs/devel/loads-stores.rst | 35 ++++++++++++++++-------------------
 8 files changed, 60 insertions(+), 35 deletions(-)

diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 18b40d6145c..2ad2d6d86bb 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -111,8 +111,6 @@ bool cpu_physical_memory_is_io(hwaddr phys_addr);
  */
 void qemu_flush_coalesced_mmio_buffer(void);
 
-void cpu_physical_memory_write_rom(AddressSpace *as, hwaddr addr,
-                                   const uint8_t *buf, int len);
 void cpu_flush_icache_range(hwaddr start, int len);
 
 extern struct MemoryRegion io_mem_rom;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 8e61450de32..ffd23ed8d8d 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1792,6 +1792,32 @@ MemTxResult address_space_write(AddressSpace *as, hwaddr addr,
                                 MemTxAttrs attrs,
                                 const uint8_t *buf, int len);
 
+/**
+ * address_space_write_rom: write to address space, including ROM.
+ *
+ * This function writes to the specified address space, but will
+ * write data to both ROM and RAM. This is used for non-guest
+ * writes like writes from the gdb debug stub or initial loading
+ * of ROM contents.
+ *
+ * Note that portions of the write which attempt to write data to
+ * a device will be silently ignored -- only real RAM and ROM will
+ * be written to.
+ *
+ * Return a MemTxResult indicating whether the operation succeeded
+ * or failed (eg unassigned memory, device rejected the transaction,
+ * IOMMU fault).
+ *
+ * @as: #AddressSpace to be accessed
+ * @addr: address within that address space
+ * @attrs: memory transaction attributes
+ * @buf: buffer with the data transferred
+ * @len: the number of bytes to write
+ */
+MemTxResult address_space_write_rom(AddressSpace *as, hwaddr addr,
+                                    MemTxAttrs attrs,
+                                    const uint8_t *buf, int len);
+
 /* address_space_ld*: load from an address space
  * address_space_st*: store to an address space
  *
diff --git a/exec.c b/exec.c
index 92679508ba3..6e875f0640a 100644
--- a/exec.c
+++ b/exec.c
@@ -3430,11 +3430,12 @@ static inline MemTxResult address_space_write_rom_internal(AddressSpace *as,
 }
 
 /* used for ROM loading : can write in RAM and ROM */
-void cpu_physical_memory_write_rom(AddressSpace *as, hwaddr addr,
-                                   const uint8_t *buf, int len)
+MemTxResult address_space_write_rom(AddressSpace *as, hwaddr addr,
+                                    MemTxAttrs attrs,
+                                    const uint8_t *buf, int len)
 {
-    address_space_write_rom_internal(as, addr, MEMTXATTRS_UNSPECIFIED,
-                                     buf, len, WRITE_DATA);
+    return address_space_write_rom_internal(as, addr, attrs,
+                                            buf, len, WRITE_DATA);
 }
 
 void cpu_flush_icache_range(hwaddr start, int len)
@@ -3879,8 +3880,9 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
             l = len;
         phys_addr += (addr & ~TARGET_PAGE_MASK);
         if (is_write) {
-            cpu_physical_memory_write_rom(cpu->cpu_ases[asidx].as,
-                                          phys_addr, buf, l);
+            address_space_write_rom(cpu->cpu_ases[asidx].as, phys_addr,
+                                    MEMTXATTRS_UNSPECIFIED,
+                                    buf, l);
         } else {
             address_space_rw(cpu->cpu_ases[asidx].as, phys_addr,
                              MEMTXATTRS_UNSPECIFIED,
diff --git a/hw/core/loader.c b/hw/core/loader.c
index aa0b3fc8679..66a616608af 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1103,8 +1103,8 @@ static void rom_reset(void *unused)
             void *host = memory_region_get_ram_ptr(rom->mr);
             memcpy(host, rom->data, rom->datasize);
         } else {
-            cpu_physical_memory_write_rom(rom->as, rom->addr, rom->data,
-                                          rom->datasize);
+            address_space_write_rom(rom->as, rom->addr, MEMTXATTRS_UNSPECIFIED,
+                                    rom->data, rom->datasize);
         }
         if (rom->isrom) {
             /* rom needs to be written only once */
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 97ffdd820f2..c9dd65b3a03 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -122,9 +122,10 @@ static void apic_sync_vapic(APICCommonState *s, int sync_type)
         }
         vapic_state.irr = vector & 0xff;
 
-        cpu_physical_memory_write_rom(&address_space_memory,
-                                      s->vapic_paddr + start,
-                                      ((void *)&vapic_state) + start, length);
+        address_space_write_rom(&address_space_memory,
+                                s->vapic_paddr + start,
+                                MEMTXATTRS_UNSPECIFIED,
+                                ((void *)&vapic_state) + start, length);
     }
 }
 
diff --git a/hw/misc/tz-mpc.c b/hw/misc/tz-mpc.c
index e0c58ba37ec..fb48a1540b9 100644
--- a/hw/misc/tz-mpc.c
+++ b/hw/misc/tz-mpc.c
@@ -448,7 +448,7 @@ static int tz_mpc_attrs_to_index(IOMMUMemoryRegion *iommu, MemTxAttrs attrs)
 {
     /* We treat unspecified attributes like secure. Transactions with
      * unspecified attributes come from places like
-     * cpu_physical_memory_write_rom() for initial image load, and we want
+     * rom_reset() for initial image load, and we want
      * those to pass through the from-reset "everything is secure" config.
      * All the real during-emulation transactions from the CPU will
      * specify attributes.
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 3c29b68e67f..639906cca30 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -559,8 +559,9 @@ static void idreg_init(hwaddr addr)
     s = SYS_BUS_DEVICE(dev);
 
     sysbus_mmio_map(s, 0, addr);
-    cpu_physical_memory_write_rom(&address_space_memory,
-                                  addr, idreg_data, sizeof(idreg_data));
+    address_space_write_rom(&address_space_memory, addr,
+                            MEMTXATTRS_UNSPECIFIED,
+                            idreg_data, sizeof(idreg_data));
 }
 
 #define MACIO_ID_REGISTER(obj) \
diff --git a/docs/devel/loads-stores.rst b/docs/devel/loads-stores.rst
index 57d8c524bfe..c74cd090e64 100644
--- a/docs/devel/loads-stores.rst
+++ b/docs/devel/loads-stores.rst
@@ -253,6 +253,22 @@ Regexes for git grep
  - ``\<address_space_ldu\?[bwql]\(_[lb]e\)\?\>``
  - ``\<address_space_st[bwql]\(_[lb]e\)\?\>``
 
+``address_space_write_rom``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+This function performs a write by physical address like
+``address_space_write``, except that if the write is to a ROM then
+the ROM contents will be modified, even though a write by the guest
+CPU to the ROM would be ignored. This is used for non-guest writes
+like writes from the gdb debug stub or initial loading of ROM contents.
+
+Note that portions of the write which attempt to write data to a
+device will be silently ignored -- only real RAM and ROM will
+be written to.
+
+Regexes for git grep
+ - ``address_space_write_rom``
+
 ``{ld,st}*_phys``
 ~~~~~~~~~~~~~~~~~
 
@@ -315,25 +331,6 @@ For new code they are better avoided:
 Regexes for git grep
  - ``\<cpu_physical_memory_\(read\|write\|rw\)\>``
 
-``cpu_physical_memory_write_rom``
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-This function performs a write by physical address like
-``address_space_write``, except that if the write is to a ROM then
-the ROM contents will be modified, even though a write by the guest
-CPU to the ROM would be ignored.
-
-Note that unlike ``cpu_physical_memory_write()`` this function takes
-an AddressSpace argument, but unlike ``address_space_write()`` this
-function does not take a ``MemTxAttrs`` or return a ``MemTxResult``.
-
-**TODO**: we should probably clean up this inconsistency and
-turn the function into ``address_space_write_rom`` with an API
-matching ``address_space_write``.
-
-``cpu_physical_memory_write_rom``
-
-
 ``cpu_memory_rw_debug``
 ~~~~~~~~~~~~~~~~~~~~~~~
 
-- 
2.19.2

[Qemu-devel] [PULL 04/22] disas.c: Use address_space_read() to read memory

[Peter Maydell]

Currently disas.c reads physical memory using
cpu_physical_memory_read(). This effectively hard-codes
assuming that all CPUs have the same view of physical
memory. Switch to address_space_read() instead, which
lets us use the AddressSpace for the CPU we're
disassembling for.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20181122172653.3413-2-peter.maydell@linaro.org
---
 disas.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/disas.c b/disas.c
index 5325b7e6be6..f9c517b3588 100644
--- a/disas.c
+++ b/disas.c
@@ -588,7 +588,10 @@ static int
 physical_read_memory(bfd_vma memaddr, bfd_byte *myaddr, int length,
                      struct disassemble_info *info)
 {
-    cpu_physical_memory_read(memaddr, myaddr, length);
+    CPUDebug *s = container_of(info, CPUDebug, info);
+
+    address_space_read(s->cpu->as, memaddr, MEMTXATTRS_UNSPECIFIED,
+                       myaddr, length);
     return 0;
 }
 
-- 
2.19.2

[Qemu-devel] [PULL 05/22] monitor: Use address_space_read() to read memory

[Peter Maydell]

Currently monitor.c reads physical memory using
cpu_physical_memory_read(). This effectively hard-codes
assuming that all CPUs have the same view of physical
memory. Switch to address_space_read() instead, which
lets us use the AddressSpace for the CPU we're
reading memory for (falling back to address_space_memory
if there is no CPU, as happens with the "none" board).
As a bonus, this allows us to detect failures to read memory.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20181122172653.3413-3-peter.maydell@linaro.org
---
 monitor.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/monitor.c b/monitor.c
index 6e81b09294e..4c8d8c2a5a0 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1605,7 +1605,13 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize,
         if (l > line_size)
             l = line_size;
         if (is_physical) {
-            cpu_physical_memory_read(addr, buf, l);
+            AddressSpace *as = cs ? cs->as : &address_space_memory;
+            MemTxResult r = address_space_read(as, addr,
+                                               MEMTXATTRS_UNSPECIFIED, buf, l);
+            if (r != MEMTX_OK) {
+                monitor_printf(mon, " Cannot access memory\n");
+                break;
+            }
         } else {
             if (cpu_memory_rw_debug(cs, addr, buf, l, 0) < 0) {
                 monitor_printf(mon, " Cannot access memory\n");
-- 
2.19.2

[Qemu-devel] [PULL 06/22] elf_ops.h: Use address_space_write() to write memory

[Peter Maydell]

Currently the load_elf function in elf_ops.h uses
cpu_physical_memory_write() to write the ELF file to
memory if it is not handling it as a ROM blob. This
means we ignore the AddressSpace that the function
is passed to define where it should be loaded.
Use address_space_write() instead.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20181122172653.3413-4-peter.maydell@linaro.org
---
 include/hw/elf_ops.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index 81cecaf27e2..74679ff8da3 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -482,7 +482,9 @@ static int glue(load_elf, SZ)(const char *name, int fd,
                     rom_add_elf_program(label, data, file_size, mem_size,
                                         addr, as);
                 } else {
-                    cpu_physical_memory_write(addr, data, file_size);
+                    address_space_write(as ? as : &address_space_memory,
+                                        addr, MEMTXATTRS_UNSPECIFIED,
+                                        data, file_size);
                     g_free(data);
                 }
             }
-- 
2.19.2

[Qemu-devel] [PULL 07/22] hw/ppc/mac_newworld, mac_oldworld: Don't use load_image()

[Peter Maydell]

The load_image() function is deprecated, as it does not let the
caller specify how large the buffer to read the file into is.
Use the glib g_file_get_contents() function instead, which does
the whole "allocate memory for the file and read it in" operation.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Message-id: 20181130151712.2312-2-peter.maydell@linaro.org
---
 hw/ppc/mac_newworld.c | 10 ++++------
 hw/ppc/mac_oldworld.c | 10 ++++------
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 14273a123e5..7e45afae7c5 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -127,8 +127,7 @@ static void ppc_core99_init(MachineState *machine)
     MACIOIDEState *macio_ide;
     BusState *adb_bus;
     MacIONVRAMState *nvr;
-    int bios_size, ndrv_size;
-    uint8_t *ndrv_file;
+    int bios_size;
     int ppc_boot_device;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     void *fw_cfg;
@@ -510,11 +509,10 @@ static void ppc_core99_init(MachineState *machine)
     /* MacOS NDRV VGA driver */
     filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, NDRV_VGA_FILENAME);
     if (filename) {
-        ndrv_size = get_image_size(filename);
-        if (ndrv_size != -1) {
-            ndrv_file = g_malloc(ndrv_size);
-            ndrv_size = load_image(filename, ndrv_file);
+        gchar *ndrv_file;
+        gsize ndrv_size;
 
+        if (g_file_get_contents(filename, &ndrv_file, &ndrv_size, NULL)) {
             fw_cfg_add_file(fw_cfg, "ndrv/qemu_vga.ndrv", ndrv_file, ndrv_size);
         }
         g_free(filename);
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 9891c325a9b..817f70e52cf 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -99,8 +99,7 @@ static void ppc_heathrow_init(MachineState *machine)
     SysBusDevice *s;
     DeviceState *dev, *pic_dev;
     BusState *adb_bus;
-    int bios_size, ndrv_size;
-    uint8_t *ndrv_file;
+    int bios_size;
     uint16_t ppc_boot_device;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     void *fw_cfg;
@@ -361,11 +360,10 @@ static void ppc_heathrow_init(MachineState *machine)
     /* MacOS NDRV VGA driver */
     filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, NDRV_VGA_FILENAME);
     if (filename) {
-        ndrv_size = get_image_size(filename);
-        if (ndrv_size != -1) {
-            ndrv_file = g_malloc(ndrv_size);
-            ndrv_size = load_image(filename, ndrv_file);
+        gchar *ndrv_file;
+        gsize ndrv_size;
 
+        if (g_file_get_contents(filename, &ndrv_file, &ndrv_size, NULL)) {
             fw_cfg_add_file(fw_cfg, "ndrv/qemu_vga.ndrv", ndrv_file, ndrv_size);
         }
         g_free(filename);
-- 
2.19.2

[Qemu-devel] [PULL 08/22] hw/ppc/ppc405_boards: Don't use load_image()

[Peter Maydell]

The load_image() function is deprecated, as it does not let the
caller specify how large the buffer to read the file into is.
Instead use load_image_size().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Message-id: 20181130151712.2312-3-peter.maydell@linaro.org
---
 hw/ppc/ppc405_boards.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 3be3fe4432b..1b0a0a8ba3a 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -219,9 +219,11 @@ static void ref405ep_init(MachineState *machine)
             bios_name = BIOS_FILENAME;
         filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
         if (filename) {
-            bios_size = load_image(filename, memory_region_get_ram_ptr(bios));
+            bios_size = load_image_size(filename,
+                                        memory_region_get_ram_ptr(bios),
+                                        BIOS_SIZE);
             g_free(filename);
-            if (bios_size < 0 || bios_size > BIOS_SIZE) {
+            if (bios_size < 0) {
                 error_report("Could not load PowerPC BIOS '%s'", bios_name);
                 exit(1);
             }
@@ -515,9 +517,11 @@ static void taihu_405ep_init(MachineState *machine)
                                &error_fatal);
         filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
         if (filename) {
-            bios_size = load_image(filename, memory_region_get_ram_ptr(bios));
+            bios_size = load_image_size(filename,
+                                        memory_region_get_ram_ptr(bios),
+                                        BIOS_SIZE);
             g_free(filename);
-            if (bios_size < 0 || bios_size > BIOS_SIZE) {
+            if (bios_size < 0) {
                 error_report("Could not load PowerPC BIOS '%s'", bios_name);
                 exit(1);
             }
-- 
2.19.2

[Qemu-devel] [PULL 09/22] hw/smbios/smbios.c: Don't use load_image()

[Peter Maydell]

The load_image() function is deprecated, as it does not let the
caller specify how large the buffer to read the file into is.
Instead use load_image_size().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20181130151712.2312-4-peter.maydell@linaro.org
---
 hw/smbios/smbios.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 920939454e7..04811279a08 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -982,7 +982,7 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
         header = (struct smbios_structure_header *)(smbios_tables +
                                                     smbios_tables_len);
 
-        if (load_image(val, (uint8_t *)header) != size) {
+        if (load_image_size(val, (uint8_t *)header, size) != size) {
             error_setg(errp, "Failed to load SMBIOS file %s", val);
             return;
         }
-- 
2.19.2

[Qemu-devel] [PULL 10/22] hw/pci/pci.c: Don't use load_image()

[Peter Maydell]

The load_image() function is deprecated, as it does not let the
caller specify how large the buffer to read the file into is.
Instead use load_image_size().

While we are converting this code, add an error-check
for read failure.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20181130151712.2312-5-peter.maydell@linaro.org
---
 hw/pci/pci.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 56b13b3320e..efb5ce196ff 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2261,7 +2261,11 @@ static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom,
     pdev->has_rom = true;
     memory_region_init_rom(&pdev->rom, OBJECT(pdev), name, size, &error_fatal);
     ptr = memory_region_get_ram_ptr(&pdev->rom);
-    load_image(path, ptr);
+    if (load_image_size(path, ptr, size) < 0) {
+        error_setg(errp, "failed to load romfile \"%s\"", pdev->romfile);
+        g_free(path);
+        return;
+    }
     g_free(path);
 
     if (is_default_rom) {
-- 
2.19.2

[Qemu-devel] [PULL 11/22] hw/i386/pc.c: Don't use load_image()

[Peter Maydell]

The load_image() function is deprecated, as it does not let the
caller specify how large the buffer to read the file into is.
Use the glib g_file_get_contents() function instead, which does
the whole "allocate memory for the file and read it in" operation.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20181130151712.2312-6-peter.maydell@linaro.org
---
 hw/i386/pc.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 4cd2fbca4d5..115bc2825ce 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -839,10 +839,9 @@ static void load_linux(PCMachineState *pcms,
 {
     uint16_t protocol;
     int setup_size, kernel_size, cmdline_size;
-    int64_t initrd_size = 0;
     int dtb_size, setup_data_offset;
     uint32_t initrd_max;
-    uint8_t header[8192], *setup, *kernel, *initrd_data;
+    uint8_t header[8192], *setup, *kernel;
     hwaddr real_addr, prot_addr, cmdline_addr, initrd_addr = 0;
     FILE *f;
     char *vmode;
@@ -965,27 +964,30 @@ static void load_linux(PCMachineState *pcms,
 
     /* load initrd */
     if (initrd_filename) {
+        gsize initrd_size;
+        gchar *initrd_data;
+        GError *gerr = NULL;
+
         if (protocol < 0x200) {
             fprintf(stderr, "qemu: linux kernel too old to load a ram disk\n");
             exit(1);
         }
 
-        initrd_size = get_image_size(initrd_filename);
-        if (initrd_size < 0) {
+        if (!g_file_get_contents(initrd_filename, &initrd_data,
+                                 &initrd_size, &gerr)) {
             fprintf(stderr, "qemu: error reading initrd %s: %s\n",
-                    initrd_filename, strerror(errno));
+                    initrd_filename, gerr->message);
             exit(1);
-        } else if (initrd_size >= initrd_max) {
+        }
+        if (initrd_size >= initrd_max) {
             fprintf(stderr, "qemu: initrd is too large, cannot support."
-                    "(max: %"PRIu32", need %"PRId64")\n", initrd_max, initrd_size);
+                    "(max: %"PRIu32", need %"PRId64")\n",
+                    initrd_max, (uint64_t)initrd_size);
             exit(1);
         }
 
         initrd_addr = (initrd_max-initrd_size) & ~4095;
 
-        initrd_data = g_malloc(initrd_size);
-        load_image(initrd_filename, initrd_data);
-
         fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_addr);
         fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
         fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, initrd_data, initrd_size);
-- 
2.19.2

[Qemu-devel] [PULL 12/22] hw/i386/multiboot.c: Don't use load_image()

[Peter Maydell]

The load_image() function is deprecated, as it does not let the
caller specify how large the buffer to read the file into is.
Instead use load_image_size().

While we are converting the code, add the missing error check.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20181130151712.2312-7-peter.maydell@linaro.org
---
 hw/i386/multiboot.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index 1a4344f5fc3..62340687e8e 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -343,7 +343,11 @@ int load_multiboot(FWCfgState *fw_cfg,
             mbs.mb_buf_size = TARGET_PAGE_ALIGN(mb_mod_length + mbs.mb_buf_size);
             mbs.mb_buf = g_realloc(mbs.mb_buf, mbs.mb_buf_size);
 
-            load_image(one_file, (unsigned char *)mbs.mb_buf + offs);
+            if (load_image_size(one_file, (unsigned char *)mbs.mb_buf + offs,
+                                mbs.mb_buf_size - offs) < 0) {
+                error_report("Error loading file '%s'", one_file);
+                exit(1);
+            }
             mb_add_mod(&mbs, mbs.mb_buf_phys + offs,
                        mbs.mb_buf_phys + offs + mb_mod_length, c);
 
-- 
2.19.2

[Qemu-devel] [PULL 13/22] hw/block/tc58128.c: Don't use load_image()

[Peter Maydell]

The load_image() function is deprecated, as it does not let the
caller specify how large the buffer to read the file into is.
Instead use load_image_size().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20181130151712.2312-8-peter.maydell@linaro.org
---
 hw/block/tc58128.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/block/tc58128.c b/hw/block/tc58128.c
index 808ad76ba60..d0fae248dcc 100644
--- a/hw/block/tc58128.c
+++ b/hw/block/tc58128.c
@@ -38,7 +38,8 @@ static void init_dev(tc58128_dev * dev, const char *filename)
     memset(dev->flash_contents, 0xff, FLASH_SIZE);
     if (filename) {
 	/* Load flash image skipping the first block */
-	ret = load_image(filename, dev->flash_contents + 528 * 32);
+        ret = load_image_size(filename, dev->flash_contents + 528 * 32,
+                              FLASH_SIZE - 528 * 32);
 	if (ret < 0) {
             if (!qtest_enabled()) {
                 error_report("Could not load flash image %s", filename);
-- 
2.19.2

[Qemu-devel] [PULL 14/22] device_tree.c: Don't use load_image()

[Peter Maydell]

The load_image() function is deprecated, as it does not let the
caller specify how large the buffer to read the file into is.
Instead use load_image_size().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20181130151712.2312-9-peter.maydell@linaro.org
---
 device_tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/device_tree.c b/device_tree.c
index 6d9c9726f66..296278e12ae 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -91,7 +91,7 @@ void *load_device_tree(const char *filename_path, int *sizep)
     /* First allocate space in qemu for device tree */
     fdt = g_malloc0(dt_size);
 
-    dt_file_load_size = load_image(filename_path, fdt);
+    dt_file_load_size = load_image_size(filename_path, fdt, dt_size);
     if (dt_file_load_size < 0) {
         error_report("Unable to open device tree file '%s'",
                      filename_path);
-- 
2.19.2

[Qemu-devel] [PULL 15/22] hw/core/loader.c: Remove load_image()

[Peter Maydell]

The load_image() function is now no longer used anywhere, so
we can remove it completely. (Use load_image_size() or
g_file_get_contents() instead.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20181130151712.2312-10-peter.maydell@linaro.org
---
 include/hw/loader.h |  1 -
 hw/core/loader.c    | 25 -------------------------
 2 files changed, 26 deletions(-)

diff --git a/include/hw/loader.h b/include/hw/loader.h
index 67a0af84ac3..3766559bc24 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -11,7 +11,6 @@
  * On error, errno is also set as appropriate.
  */
 int64_t get_image_size(const char *filename);
-int load_image(const char *filename, uint8_t *addr); /* deprecated */
 ssize_t load_image_size(const char *filename, void *addr, size_t size);
 
 /**load_image_targphys_as:
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 66a616608af..fa41842280a 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -73,31 +73,6 @@ int64_t get_image_size(const char *filename)
     return size;
 }
 
-/* return the size or -1 if error */
-/* deprecated, because caller does not specify buffer size! */
-int load_image(const char *filename, uint8_t *addr)
-{
-    int fd, size;
-    fd = open(filename, O_RDONLY | O_BINARY);
-    if (fd < 0)
-        return -1;
-    size = lseek(fd, 0, SEEK_END);
-    if (size == -1) {
-        fprintf(stderr, "file %-20s: get size error: %s\n",
-                filename, strerror(errno));
-        close(fd);
-        return -1;
-    }
-
-    lseek(fd, 0, SEEK_SET);
-    if (read(fd, addr, size) != size) {
-        close(fd);
-        return -1;
-    }
-    close(fd);
-    return size;
-}
-
 /* return the size or -1 if error */
 ssize_t load_image_size(const char *filename, void *addr, size_t size)
 {
-- 
2.19.2

[Qemu-devel] [PULL 16/22] include/hw/loader.h: Document load_image_size()

[Peter Maydell]

Add a documentation comment for load_image_size().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20181130151712.2312-11-peter.maydell@linaro.org
---
 include/hw/loader.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/hw/loader.h b/include/hw/loader.h
index 3766559bc24..0a0ad808ea3 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -11,6 +11,22 @@
  * On error, errno is also set as appropriate.
  */
 int64_t get_image_size(const char *filename);
+/**
+ * load_image_size: load an image file into specified buffer
+ * @filename: Path to the image file
+ * @addr: Buffer to load image into
+ * @size: Size of buffer in bytes
+ *
+ * Load an image file from disk into the specified buffer.
+ * If the image is larger than the specified buffer, only
+ * @size bytes are read (this is not considered an error).
+ *
+ * Prefer to use the GLib function g_file_get_contents() rather
+ * than a "get_image_size()/g_malloc()/load_image_size()" sequence.
+ *
+ * Returns the number of bytes read, or -1 on error. On error,
+ * errno is also set as appropriate.
+ */
 ssize_t load_image_size(const char *filename, void *addr, size_t size);
 
 /**load_image_targphys_as:
-- 
2.19.2

[Qemu-devel] [PULL 17/22] target/arm: Free name string in ARMCPRegInfo hashtable entries

[Peter Maydell]

When we add a new entry to the ARMCPRegInfo hash table in
add_cpreg_to_hashtable(), we allocate memory for tehe
ARMCPRegInfo struct itself, and we also g_strdup() the
name string. So the hashtable's value destructor function
must free the name string as well as the struct.

Spotted by clang's leak sanitizer. The leak here is a
small one-off leak at startup, because we don't support
CPU hotplug, and so the only time when we destroy
hash table entries is for the case where ARM_CP_OVERRIDE
means we register a wildcard entry and then override it later.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20181204132952.2601-2-peter.maydell@linaro.org
---
 target/arm/cpu.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 60411f6bfe0..b84a6c0e678 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -642,6 +642,20 @@ uint64_t arm_cpu_mp_affinity(int idx, uint8_t clustersz)
     return (Aff1 << ARM_AFF1_SHIFT) | Aff0;
 }
 
+static void cpreg_hashtable_data_destroy(gpointer data)
+{
+    /*
+     * Destroy function for cpu->cp_regs hashtable data entries.
+     * We must free the name string because it was g_strdup()ed in
+     * add_cpreg_to_hashtable(). It's OK to cast away the 'const'
+     * from r->name because we know we definitely allocated it.
+     */
+    ARMCPRegInfo *r = data;
+
+    g_free((void *)r->name);
+    g_free(r);
+}
+
 static void arm_cpu_initfn(Object *obj)
 {
     CPUState *cs = CPU(obj);
@@ -649,7 +663,7 @@ static void arm_cpu_initfn(Object *obj)
 
     cs->env_ptr = &cpu->env;
     cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
-                                         g_free, g_free);
+                                         g_free, cpreg_hashtable_data_destroy);
 
     QLIST_INIT(&cpu->pre_el_change_hooks);
     QLIST_INIT(&cpu->el_change_hooks);
-- 
2.19.2

[Qemu-devel] [PULL 18/22] hw/arm/mps2-tz.c: Free mscname string in make_dma()

[Peter Maydell]

The clang leak sanitizer spots a (one-off, trivial) memory
leak in make_dma() due to a missing free.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20181204132952.2601-3-peter.maydell@linaro.org
---
 hw/arm/mps2-tz.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index 6dd02ae47e8..82b1d020a58 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -322,6 +322,7 @@ static MemoryRegion *make_dma(MPS2TZMachineState *mms, void *opaque,
     sysbus_connect_irq(s, 2, qdev_get_gpio_in_named(iotkitdev,
                                                     "EXP_IRQ", 57 + i * 3));
 
+    g_free(mscname);
     return sysbus_mmio_get_region(s, 0);
 }
 
-- 
2.19.2

[Qemu-devel] [PULL 19/22] hw/sd/sdhci: Don't leak memory region in sdhci_sysbus_realize()

[Peter Maydell]

In sdhci_sysbus_realize() we override the initialization of
s->iomem that sdhci_common_realize() performs. However we
don't destroy the old memory region before reinitializing
it, which means that the memory allocated for mr->name in
memory_region_do_init() is leaked.

Since sdhci_initfn() already initializes s->io_ops to
&sdhci_mmio_ops, always use that in sdhci_common_realize()
and remove the now-unnecessary reinitialization of the
MMIO region from sdhci_sysbus_realize().

Spotted by clang's leak sanitizer.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20181204132952.2601-4-peter.maydell@linaro.org
---
 hw/sd/sdhci.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 81bbf032794..83f1574ffdc 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1371,7 +1371,7 @@ static void sdhci_common_realize(SDHCIState *s, Error **errp)
     s->buf_maxsz = sdhci_get_fifolen(s);
     s->fifo_buffer = g_malloc0(s->buf_maxsz);
 
-    memory_region_init_io(&s->iomem, OBJECT(s), &sdhci_mmio_ops, s, "sdhci",
+    memory_region_init_io(&s->iomem, OBJECT(s), s->io_ops, s, "sdhci",
                           SDHC_REGISTERS_MAP_SIZE);
 }
 
@@ -1565,9 +1565,6 @@ static void sdhci_sysbus_realize(DeviceState *dev, Error ** errp)
 
     sysbus_init_irq(sbd, &s->irq);
 
-    memory_region_init_io(&s->iomem, OBJECT(s), s->io_ops, s, "sdhci",
-            SDHC_REGISTERS_MAP_SIZE);
-
     sysbus_init_mmio(sbd, &s->iomem);
 }
 
-- 
2.19.2

[Qemu-devel] [PULL 20/22] tests/test-arm-mptimer: Don't leak string memory

[Peter Maydell]

The test-arm-mptimer setup creates a lot of test names using
g_strdup_printf() and never frees them. This is entirely
harmless since it's one-shot test code, but it clutters
up the output from clang's LeakSanitizer. Refactor to
use a helper function so we can free the memory.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20181204132952.2601-5-peter.maydell@linaro.org
---
 tests/test-arm-mptimer.c | 153 ++++++++++++++++++---------------------
 1 file changed, 69 insertions(+), 84 deletions(-)

diff --git a/tests/test-arm-mptimer.c b/tests/test-arm-mptimer.c
index cb8f2df9141..156a39f50dd 100644
--- a/tests/test-arm-mptimer.c
+++ b/tests/test-arm-mptimer.c
@@ -991,10 +991,25 @@ static void test_timer_zero_load_nonscaled_periodic_to_prescaled_oneshot(void)
     g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
 }
 
+/*
+ * Add a qtest test that comes in two versions: one with
+ * a timer scaler setting, and one with the timer nonscaled.
+ */
+static void add_scaler_test(const char *str, bool scale,
+                            void (*fn)(const void *))
+{
+    char *name;
+    int *scaler = scale ? &scaled : &nonscaled;
+
+    name = g_strdup_printf("%s=%d", str, *scaler);
+    qtest_add_data_func(name, scaler, fn);
+    g_free(name);
+}
+
 int main(int argc, char **argv)
 {
-    int *scaler = &nonscaled;
     int ret;
+    int scale;
 
     g_test_init(&argc, &argv, NULL);
 
@@ -1012,89 +1027,59 @@ int main(int argc, char **argv)
     qtest_add_func("mptimer/prescaler", test_timer_prescaler);
     qtest_add_func("mptimer/prescaler_on_the_fly", test_timer_prescaler_on_the_fly);
 
-tests_with_prescaler_arg:
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/oneshot scaler=%d", *scaler),
-                        scaler, test_timer_oneshot);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/pause scaler=%d", *scaler),
-                        scaler, test_timer_pause);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/reload scaler=%d", *scaler),
-                        scaler, test_timer_reload);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/periodic scaler=%d", *scaler),
-                        scaler, test_timer_periodic);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/oneshot_to_periodic scaler=%d", *scaler),
-                        scaler, test_timer_oneshot_to_periodic);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/periodic_to_oneshot scaler=%d", *scaler),
-                        scaler, test_timer_periodic_to_oneshot);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/set_oneshot_counter_to_0 scaler=%d", *scaler),
-                        scaler, test_timer_set_oneshot_counter_to_0);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/set_periodic_counter_to_0 scaler=%d", *scaler),
-                        scaler, test_timer_set_periodic_counter_to_0);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/noload_oneshot scaler=%d", *scaler),
-                        scaler, test_timer_noload_oneshot);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/noload_periodic scaler=%d", *scaler),
-                        scaler, test_timer_noload_periodic);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/zero_load_oneshot scaler=%d", *scaler),
-                        scaler, test_timer_zero_load_oneshot);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/zero_load_periodic scaler=%d", *scaler),
-                        scaler, test_timer_zero_load_periodic);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/zero_load_oneshot_to_nonzero scaler=%d", *scaler),
-                        scaler, test_timer_zero_load_oneshot_to_nonzero);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/zero_load_periodic_to_nonzero scaler=%d", *scaler),
-                        scaler, test_timer_zero_load_periodic_to_nonzero);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/nonzero_load_oneshot_to_zero scaler=%d", *scaler),
-                        scaler, test_timer_nonzero_load_oneshot_to_zero);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/nonzero_load_periodic_to_zero scaler=%d", *scaler),
-                        scaler, test_timer_nonzero_load_periodic_to_zero);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/set_periodic_counter_on_the_fly scaler=%d", *scaler),
-                        scaler, test_timer_set_periodic_counter_on_the_fly);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/enable_and_set_counter scaler=%d", *scaler),
-                        scaler, test_timer_enable_and_set_counter);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/set_counter_and_enable scaler=%d", *scaler),
-                        scaler, test_timer_set_counter_and_enable);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/oneshot_with_counter_0_on_start scaler=%d", *scaler),
-                        scaler, test_timer_oneshot_with_counter_0_on_start);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/periodic_with_counter_0_on_start scaler=%d", *scaler),
-                        scaler, test_timer_periodic_with_counter_0_on_start);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/periodic_counter scaler=%d", *scaler),
-                        scaler, test_periodic_counter);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/set_counter_periodic_with_zero_load scaler=%d", *scaler),
-                        scaler, test_timer_set_counter_periodic_with_zero_load);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/set_oneshot_load_to_0 scaler=%d", *scaler),
-                        scaler, test_timer_set_oneshot_load_to_0);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/set_periodic_load_to_0 scaler=%d", *scaler),
-                        scaler, test_timer_set_periodic_load_to_0);
-    qtest_add_data_func(
-        g_strdup_printf("mptimer/zero_load_mode_switch scaler=%d", *scaler),
-                        scaler, test_timer_zero_load_mode_switch);
-
-    if (scaler == &nonscaled) {
-        scaler = &scaled;
-        goto tests_with_prescaler_arg;
+    for (scale = 0; scale < 2; scale++) {
+        add_scaler_test("mptimer/oneshot scaler",
+                        scale, test_timer_oneshot);
+        add_scaler_test("mptimer/pause scaler",
+                        scale, test_timer_pause);
+        add_scaler_test("mptimer/reload scaler",
+                        scale, test_timer_reload);
+        add_scaler_test("mptimer/periodic scaler",
+                        scale, test_timer_periodic);
+        add_scaler_test("mptimer/oneshot_to_periodic scaler",
+                        scale, test_timer_oneshot_to_periodic);
+        add_scaler_test("mptimer/periodic_to_oneshot scaler",
+                        scale, test_timer_periodic_to_oneshot);
+        add_scaler_test("mptimer/set_oneshot_counter_to_0 scaler",
+                        scale, test_timer_set_oneshot_counter_to_0);
+        add_scaler_test("mptimer/set_periodic_counter_to_0 scaler",
+                        scale, test_timer_set_periodic_counter_to_0);
+        add_scaler_test("mptimer/noload_oneshot scaler",
+                        scale, test_timer_noload_oneshot);
+        add_scaler_test("mptimer/noload_periodic scaler",
+                        scale, test_timer_noload_periodic);
+        add_scaler_test("mptimer/zero_load_oneshot scaler",
+                        scale, test_timer_zero_load_oneshot);
+        add_scaler_test("mptimer/zero_load_periodic scaler",
+                        scale, test_timer_zero_load_periodic);
+        add_scaler_test("mptimer/zero_load_oneshot_to_nonzero scaler",
+                        scale, test_timer_zero_load_oneshot_to_nonzero);
+        add_scaler_test("mptimer/zero_load_periodic_to_nonzero scaler",
+                        scale, test_timer_zero_load_periodic_to_nonzero);
+        add_scaler_test("mptimer/nonzero_load_oneshot_to_zero scaler",
+                        scale, test_timer_nonzero_load_oneshot_to_zero);
+        add_scaler_test("mptimer/nonzero_load_periodic_to_zero scaler",
+                        scale, test_timer_nonzero_load_periodic_to_zero);
+        add_scaler_test("mptimer/set_periodic_counter_on_the_fly scaler",
+                        scale, test_timer_set_periodic_counter_on_the_fly);
+        add_scaler_test("mptimer/enable_and_set_counter scaler",
+                        scale, test_timer_enable_and_set_counter);
+        add_scaler_test("mptimer/set_counter_and_enable scaler",
+                        scale, test_timer_set_counter_and_enable);
+        add_scaler_test("mptimer/oneshot_with_counter_0_on_start scaler",
+                        scale, test_timer_oneshot_with_counter_0_on_start);
+        add_scaler_test("mptimer/periodic_with_counter_0_on_start scaler",
+                        scale, test_timer_periodic_with_counter_0_on_start);
+        add_scaler_test("mptimer/periodic_counter scaler",
+                        scale, test_periodic_counter);
+        add_scaler_test("mptimer/set_counter_periodic_with_zero_load scaler",
+                        scale, test_timer_set_counter_periodic_with_zero_load);
+        add_scaler_test("mptimer/set_oneshot_load_to_0 scaler",
+                        scale, test_timer_set_oneshot_load_to_0);
+        add_scaler_test("mptimer/set_periodic_load_to_0 scaler",
+                        scale, test_timer_set_periodic_load_to_0);
+        add_scaler_test("mptimer/zero_load_mode_switch scaler",
+                        scale, test_timer_zero_load_mode_switch);
     }
 
     qtest_start("-machine vexpress-a9");
-- 
2.19.2

[Qemu-devel] [PULL 21/22] target/arm: Create timers in realize, not init

[Peter Maydell]

The timer_new() function allocates memory; this means that
if we call it in the CPU's init method we would need
to provide an instance_finalize method to free it. Defer
the timer creation to the realize function instead.

This fixes a memory leak spotted by clang LeakSanitizer
when a CPU object is created for introspection.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20181204132952.2601-6-peter.maydell@linaro.org
---
 target/arm/cpu.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index b84a6c0e678..0e7138c9bfb 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -679,14 +679,6 @@ static void arm_cpu_initfn(Object *obj)
         qdev_init_gpio_in(DEVICE(cpu), arm_cpu_set_irq, 4);
     }
 
-    cpu->gt_timer[GTIMER_PHYS] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
-                                                arm_gt_ptimer_cb, cpu);
-    cpu->gt_timer[GTIMER_VIRT] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
-                                                arm_gt_vtimer_cb, cpu);
-    cpu->gt_timer[GTIMER_HYP] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
-                                                arm_gt_htimer_cb, cpu);
-    cpu->gt_timer[GTIMER_SEC] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
-                                                arm_gt_stimer_cb, cpu);
     qdev_init_gpio_out(DEVICE(cpu), cpu->gt_timer_outputs,
                        ARRAY_SIZE(cpu->gt_timer_outputs));
 
@@ -882,6 +874,15 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
             return;
         }
     }
+
+    cpu->gt_timer[GTIMER_PHYS] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
+                                           arm_gt_ptimer_cb, cpu);
+    cpu->gt_timer[GTIMER_VIRT] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
+                                           arm_gt_vtimer_cb, cpu);
+    cpu->gt_timer[GTIMER_HYP] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
+                                          arm_gt_htimer_cb, cpu);
+    cpu->gt_timer[GTIMER_SEC] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
+                                          arm_gt_stimer_cb, cpu);
 #endif
 
     cpu_exec_realizefn(cs, &local_err);
-- 
2.19.2

[Qemu-devel] [PULL 22/22] virt: Fix broken indentation

[Peter Maydell]

From: Eduardo Habkost <ehabkost@redhat.com>

I introduced indentation using tabs instead of spaces in another
commit.  Peter reported the problem, and I failed to fix that
before sending my pull request.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20181212003147.29604-1-ehabkost@redhat.com
Fixes: 951597607696 ("virt: Eliminate separate instance_init functions")
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/virt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 17f1b49d11f..5b678237b7d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1854,7 +1854,7 @@ static const TypeInfo virt_machine_info = {
     .instance_size = sizeof(VirtMachineState),
     .class_size    = sizeof(VirtMachineClass),
     .class_init    = virt_machine_class_init,
-	.instance_init = virt_instance_init,
+    .instance_init = virt_instance_init,
     .interfaces = (InterfaceInfo[]) {
          { TYPE_HOTPLUG_HANDLER },
          { }
-- 
2.19.2

Re: [Qemu-devel] [PULL 00/22] misc queue

[Peter Maydell]

On Fri, 14 Dec 2018 at 14:42, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This is a colletion of miscellaneous patches (mostly mine),
> which fix minor bugs or do some refactoring/cleanup.
> No user-visible changes in here.
>
> thanks
> -- PMM
>
> The following changes since commit 0f98c9945899c5dfacd5a410ff04178eda605a16:
>
>   Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2018-12-12' into staging (2018-12-14 10:19:47 +0000)
>
> are available in the Git repository at:
>
>   https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-misc-20181214
>
> for you to fetch changes up to bbac02f1e8edfd0663543f6fdad1e7094d860b29:
>
>   virt: Fix broken indentation (2018-12-14 13:30:55 +0000)
>
> ----------------------------------------------------------------
> miscellaneous patches:
>  * checkpatch.pl: Enforce multiline comment syntax
>  * Rename cpu_physical_memory_write_rom() to address_space_write_rom()
>  * disas, monitor, elf_ops: Use address_space_read() to read memory
>  * Remove load_image() in favour of load_image_size()
>  * Fix some minor memory leaks in arm boards/devices
>  * virt: fix broken indentation
>
> ----------------------------------------------------------------
> Eduardo Habkost (1):
>       virt: Fix broken indentation
>
> Peter Maydell (21):
>       scripts/checkpatch.pl: Enforce multiline comment syntax
>       exec.c: Rename cpu_physical_memory_write_rom_internal()
>       Rename cpu_physical_memory_write_rom() to address_space_write_rom()
>       disas.c: Use address_space_read() to read memory
>       monitor: Use address_space_read() to read memory
>       elf_ops.h: Use address_space_write() to write memory
>       hw/ppc/mac_newworld, mac_oldworld: Don't use load_image()
>       hw/ppc/ppc405_boards: Don't use load_image()
>       hw/smbios/smbios.c: Don't use load_image()
>       hw/pci/pci.c: Don't use load_image()
>       hw/i386/pc.c: Don't use load_image()
>       hw/i386/multiboot.c: Don't use load_image()
>       hw/block/tc58128.c: Don't use load_image()
>       device_tree.c: Don't use load_image()
>       hw/core/loader.c: Remove load_image()
>       include/hw/loader.h: Document load_image_size()
>       target/arm: Free name string in ARMCPRegInfo hashtable entries
>       hw/arm/mps2-tz.c: Free mscname string in make_dma()
>       hw/sd/sdhci: Don't leak memory region in sdhci_sysbus_realize()
>       tests/test-arm-mptimer: Don't leak string memory
>       target/arm: Create timers in realize, not init
>
>  include/exec/cpu-common.h   |   2 -
>  include/exec/memory.h       |  26 ++++++++
>  include/hw/elf_ops.h        |   4 +-
>  include/hw/loader.h         |  17 ++++-
>  device_tree.c               |   2 +-
>  disas.c                     |   5 +-
>  exec.c                      |  30 +++++----
>  hw/arm/mps2-tz.c            |   1 +
>  hw/arm/virt.c               |   2 +-
>  hw/block/tc58128.c          |   3 +-
>  hw/core/loader.c            |  29 +--------
>  hw/i386/multiboot.c         |   6 +-
>  hw/i386/pc.c                |  22 ++++---
>  hw/intc/apic.c              |   7 +-
>  hw/misc/tz-mpc.c            |   2 +-
>  hw/pci/pci.c                |   6 +-
>  hw/ppc/mac_newworld.c       |  10 ++-
>  hw/ppc/mac_oldworld.c       |  10 ++-
>  hw/ppc/ppc405_boards.c      |  12 ++--
>  hw/sd/sdhci.c               |   5 +-
>  hw/smbios/smbios.c          |   2 +-
>  hw/sparc/sun4m.c            |   5 +-
>  monitor.c                   |   8 ++-
>  target/arm/cpu.c            |  33 +++++++---
>  tests/test-arm-mptimer.c    | 153 ++++++++++++++++++++------------------------
>  docs/devel/loads-stores.rst |  35 +++++-----
>  scripts/checkpatch.pl       |  48 ++++++++++++++
>  27 files changed, 287 insertions(+), 198 deletions(-)



-- 
12345678901234567890123456789012345678901234567890123456789012345678901234567890
         1         2         3         4         5         6         7         8

Re: [Qemu-devel] [PULL 00/22] misc queue

[Peter Maydell]

On Fri, 14 Dec 2018 at 14:42, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This is a colletion of miscellaneous patches (mostly mine),
> which fix minor bugs or do some refactoring/cleanup.
> No user-visible changes in here.
>
> thanks
> -- PMM
>
> The following changes since commit 0f98c9945899c5dfacd5a410ff04178eda605a16:
>
>   Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2018-12-12' into staging (2018-12-14 10:19:47 +0000)
>
> are available in the Git repository at:
>
>   https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-misc-20181214
>
> for you to fetch changes up to bbac02f1e8edfd0663543f6fdad1e7094d860b29:
>
>   virt: Fix broken indentation (2018-12-14 13:30:55 +0000)
>
> ----------------------------------------------------------------
> miscellaneous patches:
>  * checkpatch.pl: Enforce multiline comment syntax
>  * Rename cpu_physical_memory_write_rom() to address_space_write_rom()
>  * disas, monitor, elf_ops: Use address_space_read() to read memory
>  * Remove load_image() in favour of load_image_size()
>  * Fix some minor memory leaks in arm boards/devices
>  * virt: fix broken indentation
>
Applied, thanks.
-- PMM

Re: [Qemu-devel] [PULL 00/22] misc queue

[no-reply]

Patchew URL: https://patchew.org/QEMU/20181214144214.1260-1-peter.maydell@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20181214144214.1260-1-peter.maydell@linaro.org
Type: series
Subject: [Qemu-devel] [PULL 00/22] misc queue

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
a270d9d virt: Fix broken indentation
cbd1ee0 target/arm: Create timers in realize, not init
dc7dad8 tests/test-arm-mptimer: Don't leak string memory
515636e hw/sd/sdhci: Don't leak memory region in sdhci_sysbus_realize()
e774828 hw/arm/mps2-tz.c: Free mscname string in make_dma()
3ddd560 target/arm: Free name string in ARMCPRegInfo hashtable entries
98bd688 include/hw/loader.h: Document load_image_size()
8cc4663 hw/core/loader.c: Remove load_image()
200adc0 device_tree.c: Don't use load_image()
a168dfe hw/block/tc58128.c: Don't use load_image()
03e819b hw/i386/multiboot.c: Don't use load_image()
7773750 hw/i386/pc.c: Don't use load_image()
deef451 hw/pci/pci.c: Don't use load_image()
bc9b210 hw/smbios/smbios.c: Don't use load_image()
8561b4f hw/ppc/ppc405_boards: Don't use load_image()
6fa8785 hw/ppc/mac_newworld, mac_oldworld: Don't use load_image()
c2fa852 elf_ops.h: Use address_space_write() to write memory
6b5a590 monitor: Use address_space_read() to read memory
204baf1 disas.c: Use address_space_read() to read memory
5b75a36 Rename cpu_physical_memory_write_rom() to address_space_write_rom()
247aebb exec.c: Rename cpu_physical_memory_write_rom_internal()
9230e2d scripts/checkpatch.pl: Enforce multiline comment syntax

=== OUTPUT BEGIN ===
Checking PATCH 1/22: scripts/checkpatch.pl: Enforce multiline comment syntax...
WARNING: line over 80 characters
#44: FILE: scripts/checkpatch.pl:1576:
+                   $rawline =~ m@^\+.*/\*\*?[ \t]*.+[ \t]*$@) { # /* or /** non-blank

ERROR: line over 90 characters
#45: FILE: scripts/checkpatch.pl:1577:
+                       WARN("Block comments use a leading /* on a separate line\n" . $herecurr);

WARNING: line over 80 characters
#54: FILE: scripts/checkpatch.pl:1586:
+                       WARN("Block comments use * on subsequent lines\n" . $hereprev);

ERROR: line over 90 characters
#62: FILE: scripts/checkpatch.pl:1594:
+                       WARN("Block comments use a trailing */ on a separate line\n" . $herecurr);

ERROR: line over 90 characters
#84: FILE: scripts/checkpatch.pl:1616:
+                               WARN("Block comments should align the * on each line\n" . $hereprev);

total: 3 errors, 2 warnings, 54 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/22: exec.c: Rename cpu_physical_memory_write_rom_internal()...
WARNING: line over 80 characters
#39: FILE: exec.c:3396:
+                                                           enum write_rom_type type)

total: 0 errors, 1 warnings, 50 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 3/22: Rename cpu_physical_memory_write_rom() to address_space_write_rom()...
WARNING: Block comments use a leading /* on a separate line
#190: FILE: include/exec/memory.h:1795:
+/**

total: 0 errors, 1 warnings, 156 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 4/22: disas.c: Use address_space_read() to read memory...
Checking PATCH 5/22: monitor: Use address_space_read() to read memory...
Checking PATCH 6/22: elf_ops.h: Use address_space_write() to write memory...
Checking PATCH 7/22: hw/ppc/mac_newworld, mac_oldworld: Don't use load_image()...
Checking PATCH 8/22: hw/ppc/ppc405_boards: Don't use load_image()...
Checking PATCH 9/22: hw/smbios/smbios.c: Don't use load_image()...
Checking PATCH 10/22: hw/pci/pci.c: Don't use load_image()...
Checking PATCH 11/22: hw/i386/pc.c: Don't use load_image()...
Checking PATCH 12/22: hw/i386/multiboot.c: Don't use load_image()...
Checking PATCH 13/22: hw/block/tc58128.c: Don't use load_image()...
Checking PATCH 14/22: device_tree.c: Don't use load_image()...
Checking PATCH 15/22: hw/core/loader.c: Remove load_image()...
Checking PATCH 16/22: include/hw/loader.h: Document load_image_size()...
WARNING: Block comments use a leading /* on a separate line
#24: FILE: include/hw/loader.h:14:
+/**

total: 0 errors, 1 warnings, 22 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 17/22: target/arm: Free name string in ARMCPRegInfo hashtable entries...
Checking PATCH 18/22: hw/arm/mps2-tz.c: Free mscname string in make_dma()...
Checking PATCH 19/22: hw/sd/sdhci: Don't leak memory region in sdhci_sysbus_realize()...
Checking PATCH 20/22: tests/test-arm-mptimer: Don't leak string memory...
Checking PATCH 21/22: target/arm: Create timers in realize, not init...
Checking PATCH 22/22: virt: Fix broken indentation...
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20181214144214.1260-1-peter.maydell@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com