qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2 RFC] Implement Hex file loader and add test case
@ 2018-04-03 15:17 Su Hang
  2018-04-03 15:25 ` no-reply
  0 siblings, 1 reply; 8+ messages in thread
From: Su Hang @ 2018-04-03 15:17 UTC (permalink / raw)
  To: stefanha, jim, joel, qemu-devel

These series of patchs implement Intel Hexadecimal File loader and
add QTest testcase to verify the correctness of Loader.  

Su Hang (2):
  Implement .hex file loader
  Add QTest testcase for the Intel Hexadecimal Object File Loader.

 hw/arm/boot.c          |   9 +-
 hw/core/loader.c       | 280 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/loader.h    |  17 +++
 tests/Makefile.include |   2 +
 tests/hexloader-test.c |  56 ++++++++++
 tests/test.hex         |  11 ++
 6 files changed, 374 insertions(+), 1 deletion(-)
 create mode 100644 tests/hexloader-test.c
 create mode 100644 tests/test.hex

-- 
2.7.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2 RFC] Implement Hex file loader and add test case
  2018-04-03 15:17 Su Hang
@ 2018-04-03 15:25 ` no-reply
  0 siblings, 0 replies; 8+ messages in thread
From: no-reply @ 2018-04-03 15:25 UTC (permalink / raw)
  To: suhang16; +Cc: famz, stefanha, jim, joel, qemu-devel

Hi,

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

Type: series
Message-id: 1522768634-5548-1-git-send-email-suhang16@mails.ucas.ac.cn
Subject: [Qemu-devel] [PATCH 0/2 RFC] Implement Hex file loader and add test case

=== 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
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/1522768634-5548-1-git-send-email-suhang16@mails.ucas.ac.cn -> patchew/1522768634-5548-1-git-send-email-suhang16@mails.ucas.ac.cn
Switched to a new branch 'test'
ae6e4ea631 Add QTest testcase for the Hex File Loader
3392bbf17c Implement .hex file loader

=== OUTPUT BEGIN ===
Checking PATCH 1/2: Implement .hex file loader...
ERROR: braces {} are necessary for all arms of this statement
#220: FILE: hw/core/loader.c:1463:
+    } while (++hex_blob != end);
[...]

total: 1 errors, 0 warnings, 328 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/2: Add QTest testcase for the Hex File Loader...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 0/2 RFC] Implement Hex file loader and add test case
@ 2018-04-03 15:30 Su Hang
  2018-04-03 15:30 ` [Qemu-devel] [PATCH 1/2] Implement .hex file loader Su Hang
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Su Hang @ 2018-04-03 15:30 UTC (permalink / raw)
  To: stefanha, jim, joel, qemu-devel

These series of patchs implement Intel Hexadecimal File loader and
add QTest testcase to verify the correctness of Loader.  

Su Hang (2):
  Implement .hex file loader
  Add QTest testcase for the Intel Hexadecimal Object File Loader.

 hw/arm/boot.c          |   9 +-
 hw/core/loader.c       | 280 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/loader.h    |  17 +++
 tests/Makefile.include |   2 +
 tests/hexloader-test.c |  56 ++++++++++
 tests/test.hex         |  11 ++
 6 files changed, 374 insertions(+), 1 deletion(-)
 create mode 100644 tests/hexloader-test.c
 create mode 100644 tests/test.hex

-- 
2.7.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel]  [PATCH 1/2] Implement .hex file loader
  2018-04-03 15:30 [Qemu-devel] [PATCH 0/2 RFC] Implement Hex file loader and add test case Su Hang
@ 2018-04-03 15:30 ` Su Hang
  2018-04-03 15:30 ` [Qemu-devel] [PATCH 2/2] Add QTest testcase for the Hex File Loader Su Hang
  2018-04-03 15:37 ` [Qemu-devel] [PATCH 0/2 RFC] Implement Hex file loader and add test case no-reply
  2 siblings, 0 replies; 8+ messages in thread
From: Su Hang @ 2018-04-03 15:30 UTC (permalink / raw)
  To: stefanha, jim, joel, qemu-devel

This patch adds Intel Hexadecimal Object File format support to
the loader.  The file format specification is available here:
http://www.piclist.com/techref/fileext/hex/intel.htm

The file format is mainly intended for embedded systems
and microcontrollers, such as Arduino, ARM, STM32, etc.

Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Su Hang <suhang16@mails.ucas.ac.cn>
---
 hw/arm/boot.c       |   9 +-
 hw/core/loader.c    | 280 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/loader.h |  17 ++++
 3 files changed, 305 insertions(+), 1 deletion(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 9319b12fcd2a..07ce54e5936d 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -1060,8 +1060,15 @@ static void arm_load_kernel_notify(Notifier *notifier, void *data)
         kernel_size = load_aarch64_image(info->kernel_filename,
                                          info->loader_start, &entry, as);
         is_linux = 1;
+    } else if (kernel_size < 0 && strstr(info->kernel_filename, ".hex")) {
+        /* 32-bit ARM .hex file */
+        entry = info->loader_start + KERNEL_LOAD_ADDR;
+        kernel_size = load_targphys_hex_as(info->kernel_filename, entry,
+                                           info->ram_size - KERNEL_LOAD_ADDR,
+                                           as);
+        is_linux = 1;
     } else if (kernel_size < 0) {
-        /* 32-bit ARM */
+        /* 32-bit ARM binary file */
         entry = info->loader_start + KERNEL_LOAD_ADDR;
         kernel_size = load_image_targphys_as(info->kernel_filename, entry,
                                              info->ram_size - KERNEL_LOAD_ADDR,
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 06bdbca53709..126832c4243c 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1286,3 +1286,283 @@ void hmp_info_roms(Monitor *mon, const QDict *qdict)
         }
     }
 }
+
+typedef enum HexRecord HexRecord;
+enum HexRecord {
+    DATA_RECORD = 0,
+    EOF_RECORD,
+    EXT_SEG_ADDR_RECORD,
+    START_SEG_ADDR_RECORD,
+    EXT_LINEAR_ADDR_RECORD,
+    START_LINEAR_ADDR_RECORD,
+};
+
+typedef union HexLine HexLine;
+union HexLine {
+    uint8_t buf[0x25];
+    struct __attribute__((packed)) {
+        uint8_t byte_count;
+        uint16_t address;
+        uint8_t record_type;
+        uint8_t data[0x25 - 0x5];
+        uint8_t checksum;
+    };
+};
+
+static uint8_t ctoh(char c)
+{
+    return (c & 0x10) ? /*0-9*/ c & 0xf : /*A-F, a-f*/ (c & 0xf) + 9;
+}
+
+static uint8_t validate_checksum(HexLine *record)
+{
+    uint8_t result = 0, i = 0;
+
+    for (; i < (record->byte_count + 5); ++i) {
+        result += record->buf[i];
+    }
+
+    return result == 0;
+}
+
+/* return pointer of bin_blob or NULL if error */
+static uint8_t *parse_hex_blob(char *filename, size_t *p_size)
+{
+    int fd;
+    off_t hex_blob_size;
+    uint8_t *p_data = NULL;
+    uint8_t *hex_blob;
+    uint8_t *hex_blob_ori;         /* used to free temporary memory */
+    uint8_t *bin_buf;
+    uint8_t *end;
+    uint8_t idx = 0;
+    uint8_t in_process = 0;        /* avoid re-enter */
+    uint8_t low_nibble = 0;        /* process two hex char into 8-bits */
+    uint8_t ext_linear_record = 0; /* record non-constitutes block */
+    uint32_t next_address_to_write = 0;
+    uint32_t current_address = 0;
+    uint32_t last_address = 0;
+    HexLine line = {0};
+
+    fd = open(filename, O_RDONLY);
+    if (fd < 0) {
+        return NULL;
+    }
+    hex_blob_size = lseek(fd, 0, SEEK_END);
+    if (hex_blob_size < 0) {
+        close(fd);
+        return NULL;
+    }
+    hex_blob = g_malloc(hex_blob_size);
+    hex_blob_ori = hex_blob;
+    bin_buf = g_malloc(hex_blob_size * 2);
+    lseek(fd, 0, SEEK_SET);
+    if (read(fd, hex_blob, hex_blob_size) != hex_blob_size) {
+        close(fd);
+        goto hex_parser_exit;
+    }
+    close(fd);
+
+    memset(line.buf, 0, sizeof(HexLine));
+    end = (uint8_t *)hex_blob + hex_blob_size;
+
+    do {
+        switch ((uint8_t)(*hex_blob)) {
+        case '\r':
+        case '\n':
+            if (!in_process) {
+                break;
+            }
+
+            in_process = 0;
+            if (validate_checksum(&line) == 0) {
+                p_data = NULL;
+                goto hex_parser_exit;
+            }
+
+            line.address = bswap16(line.address);
+            switch (line.record_type) {
+            case DATA_RECORD:
+                current_address =
+                    (next_address_to_write & 0xffff0000) | line.address;
+                /* verify this is a continous block of memory */
+                if (current_address != next_address_to_write ||
+                    ext_linear_record) {
+                    if (!ext_linear_record) {
+                        /* Store next address to write */
+                        last_address = next_address_to_write;
+                        next_address_to_write = current_address;
+                    }
+                    ext_linear_record = 0;
+                    memset(bin_buf + last_address, 0x0,
+                           current_address - last_address);
+                }
+
+                /* copy from line buffer to output bin_buf */
+                memcpy((uint8_t *)bin_buf + current_address,
+                       (uint8_t *)line.data, line.byte_count);
+                /* Save next address to write */
+                last_address = current_address;
+                next_address_to_write = current_address + line.byte_count;
+                break;
+
+            case EOF_RECORD:
+                /* nothing to do */
+                break;
+            case EXT_SEG_ADDR_RECORD:
+                /* save next address to write,
+                 * in case of non-continous block of memory */
+                ext_linear_record = 1;
+                last_address = next_address_to_write;
+                next_address_to_write =
+                    ((line.data[0] << 12) | (line.data[1] << 4));
+                break;
+            case START_SEG_ADDR_RECORD:
+                /* TODO */
+                break;
+
+            case EXT_LINEAR_ADDR_RECORD:
+                /* save next address to write,
+                 * in case of non-continous block of memory */
+                ext_linear_record = 1;
+                last_address = next_address_to_write;
+                next_address_to_write =
+                    ((line.data[0] << 24) | (line.data[1] << 16));
+                break;
+            case START_LINEAR_ADDR_RECORD:
+                /* TODO */
+                break;
+
+            default:
+                p_data = NULL;
+                goto hex_parser_exit;
+            }
+            break;
+
+        /* start of a new record. */
+        case ':':
+            memset(line.buf, 0, sizeof(HexLine));
+            in_process = 1;
+            low_nibble = 0;
+            idx = 0;
+            break;
+
+        /* decoding lines */
+        default:
+            if (low_nibble) {
+                line.buf[idx] |= ctoh((uint8_t)(*hex_blob)) & 0xf;
+                ++idx;
+            } else {
+                line.buf[idx] = ctoh((uint8_t)(*hex_blob)) << 4;
+            }
+
+            low_nibble = !low_nibble;
+            break;
+        }
+
+    } while (++hex_blob != end);
+
+    *p_size = (size_t)next_address_to_write;
+    p_data = g_malloc(next_address_to_write);
+
+    memcpy(p_data, bin_buf, next_address_to_write);
+hex_parser_exit:
+    g_free(hex_blob_ori);
+    g_free(bin_buf);
+    return p_data;
+}
+
+/* return size or -1 if error */
+int load_targphys_hex_as(const char *filename, hwaddr addr, uint64_t max_sz,
+                         AddressSpace *as)
+{
+    return rom_add_hex_file(filename, NULL, addr, -1, false, NULL, as);
+}
+
+/* return size -1 if error */
+int rom_add_hex_file(const char *file, const char *fw_dir, hwaddr addr,
+                     int32_t bootindex, bool option_rom, MemoryRegion *mr,
+                     AddressSpace *as)
+{
+    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
+    Rom *rom;
+    char devpath[100];
+    size_t datasize = 0;
+
+    if (as && mr) {
+        fprintf(stderr, "Specifying an Address Space and Memory Region is "
+                        "not valid when loading a rom\n");
+        /* We haven't allocated anything so we don't need any cleanup */
+        return -1;
+    }
+
+    rom = g_malloc0(sizeof(*rom));
+    rom->name = g_strdup(file);
+    rom->path = qemu_find_file(QEMU_FILE_TYPE_BIOS, rom->name);
+    rom->as = as;
+    if (rom->path == NULL) {
+        rom->path = g_strdup(file);
+    }
+
+    rom->data = parse_hex_blob(rom->path, &datasize);
+    if (rom->data == NULL) {
+        fprintf(stderr, "failed to parse hex file '%s': %s\n", rom->path,
+                strerror(errno));
+        goto err;
+    }
+    rom->datasize = datasize;
+
+    if (fw_dir) {
+        rom->fw_dir = g_strdup(fw_dir);
+        rom->fw_file = g_strdup(file);
+    }
+    rom->addr = addr;
+    rom->romsize = rom->datasize;
+
+    rom_insert(rom);
+
+    if (rom->fw_file && fw_cfg) {
+        const char *basename;
+        char fw_file_name[FW_CFG_MAX_FILE_PATH];
+        void *data;
+
+        basename = strrchr(rom->fw_file, '/');
+        if (basename) {
+            ++basename;
+        } else {
+            basename = rom->fw_file;
+        }
+        snprintf(fw_file_name, sizeof(fw_file_name), "%s/%s", rom->fw_dir,
+                 basename);
+        snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name);
+
+        if ((!option_rom || mc->option_rom_has_mr) && mc->rom_file_has_mr) {
+            data = rom_set_mr(rom, OBJECT(fw_cfg), devpath, true);
+        } else {
+            data = rom->data;
+        }
+
+        fw_cfg_add_file(fw_cfg, fw_file_name, data, rom->romsize);
+    } else {
+        if (mr) {
+            rom->mr = mr;
+            snprintf(devpath, sizeof(devpath), "/rom@%s", file);
+        } else {
+            snprintf(devpath, sizeof(devpath), "/rom@" TARGET_FMT_plx, addr);
+        }
+    }
+
+    add_boot_device_path(bootindex, NULL, devpath);
+    return rom->datasize;
+
+err:
+    g_free(rom->path);
+    g_free(rom->name);
+    if (fw_dir) {
+        g_free(rom->fw_dir);
+        g_free(rom->fw_file);
+    }
+    g_free(rom);
+
+    return -1;
+}
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 5ed3fd8ae67a..176b11221a27 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -28,6 +28,20 @@ ssize_t load_image_size(const char *filename, void *addr, size_t size);
 int load_image_targphys_as(const char *filename,
                            hwaddr addr, uint64_t max_sz, AddressSpace *as);
 
+/**load_image_targphys_hex_as:
+ * @filename: Path to the .hex file
+ * @addr: Address to load the .hex file to
+ * @max_sz: The maximum size of the .hex file to load
+ * @as: The AddressSpace to load the .hex file to. The value of
+ *      address_space_memory is used if nothing is supplied here.
+ *
+ * Load a fixed .hex file into memory.
+ *
+ * Returns the size of the loaded .hex file on success, -1 otherwise.
+ */
+int load_targphys_hex_as(const char *filename,
+                         hwaddr addr, uint64_t max_sz, AddressSpace *as);
+
 /** load_image_targphys:
  * Same as load_image_targphys_as(), but doesn't allow the caller to specify
  * an AddressSpace.
@@ -210,6 +224,9 @@ void pstrcpy_targphys(const char *name,
 extern bool option_rom_has_mr;
 extern bool rom_file_has_mr;
 
+int rom_add_hex_file(const char *file, const char *fw_dir,
+                 hwaddr addr, int32_t bootindex,
+                 bool option_rom, MemoryRegion *mr, AddressSpace *as);
 int rom_add_file(const char *file, const char *fw_dir,
                  hwaddr addr, int32_t bootindex,
                  bool option_rom, MemoryRegion *mr, AddressSpace *as);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 2/2] Add QTest testcase for the Hex File Loader
  2018-04-03 15:30 [Qemu-devel] [PATCH 0/2 RFC] Implement Hex file loader and add test case Su Hang
  2018-04-03 15:30 ` [Qemu-devel] [PATCH 1/2] Implement .hex file loader Su Hang
@ 2018-04-03 15:30 ` Su Hang
  2018-04-03 15:37 ` [Qemu-devel] [PATCH 0/2 RFC] Implement Hex file loader and add test case no-reply
  2 siblings, 0 replies; 8+ messages in thread
From: Su Hang @ 2018-04-03 15:30 UTC (permalink / raw)
  To: stefanha, jim, joel, qemu-devel

'test.hex' file is a bare metal ARM software stored in Hexadecimal
Object Format. When it's loaded by QEMU, it will print "Hello world!\n"
on console.

`pre_store` array in 'hexloader-test.c' file, stores the binary format
of 'test.hex' file, which is used to verify correctness.

Signed-off-by: Su Hang <suhang16@mails.ucas.ac.cn>
---
 tests/Makefile.include |  2 ++
 tests/hexloader-test.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/test.hex         | 11 ++++++++++
 3 files changed, 69 insertions(+)
 create mode 100644 tests/hexloader-test.c
 create mode 100644 tests/test.hex

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3b9a5e31a2c2..f4a3e71f34ee 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -380,6 +380,7 @@ check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF)
 gcov-files-arm-y += hw/timer/arm_mptimer.c
 check-qtest-arm-y += tests/boot-serial-test$(EXESUF)
 check-qtest-arm-y += tests/sdhci-test$(EXESUF)
+check-qtest-arm-y += tests/hexloader-test$(EXESUF)
 
 check-qtest-aarch64-y = tests/numa-test$(EXESUF)
 check-qtest-aarch64-y += tests/sdhci-test$(EXESUF)
@@ -755,6 +756,7 @@ tests/qmp-test$(EXESUF): tests/qmp-test.o
 tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o
 tests/rtc-test$(EXESUF): tests/rtc-test.o
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o
+tests/hexloader-test$(EXESUF): tests/hexloader-test.o
 tests/endianness-test$(EXESUF): tests/endianness-test.o
 tests/spapr-phb-test$(EXESUF): tests/spapr-phb-test.o $(libqos-obj-y)
 tests/prom-env-test$(EXESUF): tests/prom-env-test.o $(libqos-obj-y)
diff --git a/tests/hexloader-test.c b/tests/hexloader-test.c
new file mode 100644
index 000000000000..b1a491dcd9de
--- /dev/null
+++ b/tests/hexloader-test.c
@@ -0,0 +1,56 @@
+/*
+ * QTest testcase for the Intel Hexadecimal Object File Loader
+ *
+ * Authors:
+ *  Su Hang <suhang16@mails.ucas.ac.cn> 2018
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+
+#define BIN_SIZE 146
+
+static unsigned char pre_store[BIN_SIZE] = {
+    4,   208, 159, 229, 22,  0,   0,   235, 254, 255, 255, 234, 152, 16,  1,
+    0,   4,   176, 45,  229, 0,   176, 141, 226, 12,  208, 77,  226, 8,   0,
+    11,  229, 6,   0,   0,   234, 8,   48,  27,  229, 0,   32,  211, 229, 44,
+    48,  159, 229, 0,   32,  131, 229, 8,   48,  27,  229, 1,   48,  131, 226,
+    8,   48,  11,  229, 8,   48,  27,  229, 0,   48,  211, 229, 0,   0,   83,
+    227, 244, 255, 255, 26,  0,   0,   160, 225, 0,   208, 139, 226, 4,   176,
+    157, 228, 30,  255, 47,  225, 0,   16,  31,  16,  0,   72,  45,  233, 4,
+    176, 141, 226, 8,   0,   159, 229, 230, 255, 255, 235, 0,   0,   160, 225,
+    0,   136, 189, 232, 132, 0,   1,   0,   0,   16,  31,  16,  72,  101, 108,
+    108, 111, 32,  119, 111, 114, 108, 100, 33,  10,  0};
+
+/* success if no crash or abort */
+static void hex_loader_test(void)
+{
+    unsigned int i;
+    unsigned char memory_content[BIN_SIZE];
+    const unsigned int base_addr = 0x00010000;
+
+    QTestState *s = qtest_startf(
+        "-M versatilepb -m 128M -nographic -kernel ../tests/test.hex");
+
+    for (i = 0; i < BIN_SIZE; ++i) {
+        memory_content[i] = qtest_readb(s, base_addr + i);
+        g_assert_cmpuint(memory_content[i], ==, pre_store[i]);
+    }
+    qtest_quit(s);
+}
+
+int main(int argc, char **argv)
+{
+    int ret;
+
+    g_test_init(&argc, &argv, NULL);
+
+    qtest_add_func("/tmp/hex_loader", hex_loader_test);
+    ret = g_test_run();
+
+    return ret;
+}
diff --git a/tests/test.hex b/tests/test.hex
new file mode 100644
index 000000000000..59b96e3e6fa7
--- /dev/null
+++ b/tests/test.hex
@@ -0,0 +1,11 @@
+:1000000004D09FE5160000EBFEFFFFEA9810010008
+:1000100004B02DE500B08DE20CD04DE208000BE5F8
+:10002000060000EA08301BE50020D3E52C309FE5F0
+:10003000002083E508301BE5013083E208300BE542
+:1000400008301BE50030D3E5000053E3F4FFFF1A4E
+:100050000000A0E100D08BE204B09DE41EFF2FE180
+:1000600000101F1000482DE904B08DE208009FE544
+:10007000E6FFFFEB0000A0E10088BDE8840001007E
+:1000800000101F1048656C6C6F20776F726C6421D4
+:020090000A0064
+:00000001FF
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2 RFC] Implement Hex file loader and add test case
  2018-04-03 15:30 [Qemu-devel] [PATCH 0/2 RFC] Implement Hex file loader and add test case Su Hang
  2018-04-03 15:30 ` [Qemu-devel] [PATCH 1/2] Implement .hex file loader Su Hang
  2018-04-03 15:30 ` [Qemu-devel] [PATCH 2/2] Add QTest testcase for the Hex File Loader Su Hang
@ 2018-04-03 15:37 ` no-reply
  2018-04-04 15:45   ` Eric Blake
  2 siblings, 1 reply; 8+ messages in thread
From: no-reply @ 2018-04-03 15:37 UTC (permalink / raw)
  To: suhang16; +Cc: famz, stefanha, jim, joel, qemu-devel

Hi,

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

Type: series
Message-id: 1522769426-6056-1-git-send-email-suhang16@mails.ucas.ac.cn
Subject: [Qemu-devel] [PATCH 0/2 RFC] Implement Hex file loader and add test case

=== 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
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/1522769426-6056-1-git-send-email-suhang16@mails.ucas.ac.cn -> patchew/1522769426-6056-1-git-send-email-suhang16@mails.ucas.ac.cn
Switched to a new branch 'test'
3745c78bdd Add QTest testcase for the Hex File Loader
ef49b9408b Implement .hex file loader

=== OUTPUT BEGIN ===
Checking PATCH 1/2: Implement .hex file loader...
ERROR: braces {} are necessary for all arms of this statement
#220: FILE: hw/core/loader.c:1463:
+    } while (++hex_blob != end);
[...]

total: 1 errors, 0 warnings, 328 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/2: Add QTest testcase for the Hex File Loader...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2 RFC] Implement Hex file loader and add test case
  2018-04-03 15:37 ` [Qemu-devel] [PATCH 0/2 RFC] Implement Hex file loader and add test case no-reply
@ 2018-04-04 15:45   ` Eric Blake
  2018-04-04 16:00     ` Su Hang
  0 siblings, 1 reply; 8+ messages in thread
From: Eric Blake @ 2018-04-04 15:45 UTC (permalink / raw)
  To: qemu-devel, no-reply, suhang16; +Cc: stefanha, jim, famz, joel

[-- Attachment #1: Type: text/plain, Size: 749 bytes --]

On 04/03/2018 10:37 AM, no-reply@patchew.org wrote:
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 

> === OUTPUT BEGIN ===
> Checking PATCH 1/2: Implement .hex file loader...
> ERROR: braces {} are necessary for all arms of this statement
> #220: FILE: hw/core/loader.c:1463:
> +    } while (++hex_blob != end);
> [...]

False positive; this is a known bug in checkpatch.pl, and there is even
a patch proposed to resolve it, but no one has incorporated the patch yet:

https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg05232.html

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


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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2 RFC] Implement Hex file loader and add test case
  2018-04-04 15:45   ` Eric Blake
@ 2018-04-04 16:00     ` Su Hang
  0 siblings, 0 replies; 8+ messages in thread
From: Su Hang @ 2018-04-04 16:00 UTC (permalink / raw)
  To: Eric Blake; +Cc: qemu-devel, no-reply, stefanha, jim, famz, joel

A bit embarrassing, this mistake was introduced by me. I submitted a corresponding patch to fix it, but no one has reviewed it.
See link below:
http://lists.nongnu.org/archive/html/qemu-devel/2018-03/msg06506.html

"Eric Blake" <eblake@redhat.com>wrote:
> On 04/03/2018 10:37 AM, no-reply@patchew.org wrote:
> > Hi,
> > 
> > This series seems to have some coding style problems. See output below for
> > more information:
> > 
> 
> > === OUTPUT BEGIN ===
> > Checking PATCH 1/2: Implement .hex file loader...
> > ERROR: braces {} are necessary for all arms of this statement
> > #220: FILE: hw/core/loader.c:1463:
> > +    } while (++hex_blob != end);
> > [...]
> 
> False positive; this is a known bug in checkpatch.pl, and there is even
> a patch proposed to resolve it, but no one has incorporated the patch yet:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg05232.html
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-04-04 16:00 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-04-03 15:30 [Qemu-devel] [PATCH 0/2 RFC] Implement Hex file loader and add test case Su Hang
2018-04-03 15:30 ` [Qemu-devel] [PATCH 1/2] Implement .hex file loader Su Hang
2018-04-03 15:30 ` [Qemu-devel] [PATCH 2/2] Add QTest testcase for the Hex File Loader Su Hang
2018-04-03 15:37 ` [Qemu-devel] [PATCH 0/2 RFC] Implement Hex file loader and add test case no-reply
2018-04-04 15:45   ` Eric Blake
2018-04-04 16:00     ` Su Hang
  -- strict thread matches above, loose matches on Subject: below --
2018-04-03 15:17 Su Hang
2018-04-03 15:25 ` no-reply

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).