Re: [PATCH v2 02/39] tests/qtest: i440fx-test: Rewrite create_blob_file() to be portable

["Marc-André Lureau" <marcandre.lureau@gmail.com>]

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

Hi

On Tue, Sep 20, 2022 at 12:56 PM Bin Meng <bmeng.cn@gmail.com> wrote:

> From: Bin Meng <bin.meng@windriver.com>
>
> Previously request_{bios, pflash} cases were skipped on win32, mainly
> due to create_blob_file() calling mmap() which does not exist on win32.
> This rewirtes create_blob_file() to be portable, so that we can enable
> these cases on Windows.
>
> Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
> Changes in v2:
> - new patch: "tests/qtest: i440fx-test: Rewrite create_blob_file() to be
> portable"
>
>  tests/qtest/i440fx-test.c | 53 +++++++++++----------------------------
>  1 file changed, 14 insertions(+), 39 deletions(-)
>
> diff --git a/tests/qtest/i440fx-test.c b/tests/qtest/i440fx-test.c
> index 3890f1237c..202bc6022c 100644
> --- a/tests/qtest/i440fx-test.c
> +++ b/tests/qtest/i440fx-test.c
> @@ -278,8 +278,6 @@ static void test_i440fx_pam(gconstpointer opaque)
>      qtest_end();
>  }
>
> -#ifndef _WIN32
> -
>  #define BLOB_SIZE ((size_t)65536)
>  #define ISA_BIOS_MAXSZ ((size_t)(128 * 1024))
>
> @@ -290,44 +288,25 @@ static void test_i440fx_pam(gconstpointer opaque)
>   */
>  static char *create_blob_file(void)
>  {
> -    int ret, fd;
> +    int i, fd;
>      char *pathname;
> -    GError *error = NULL;
> +    GError *error;
>

Bad change, please keep "error = NULL".

+    g_autofree uint8_t *buf = g_malloc(BLOB_SIZE);
>
> -    ret = -1;
> +    error = NULL;
>

Not necessary then


>      fd = g_file_open_tmp("blob_XXXXXX", &pathname, &error);
> -    if (fd == -1) {
> -        fprintf(stderr, "unable to create blob file: %s\n",
> error->message);
> -        g_error_free(error);
> -    } else {
> -        if (ftruncate(fd, BLOB_SIZE) == -1) {
> -            fprintf(stderr, "ftruncate(\"%s\", %zu): %s\n", pathname,
> -                    BLOB_SIZE, strerror(errno));
> -        } else {
> -            void *buf;
> -
> -            buf = mmap(NULL, BLOB_SIZE, PROT_WRITE, MAP_SHARED, fd, 0);
> -            if (buf == MAP_FAILED) {
> -                fprintf(stderr, "mmap(\"%s\", %zu): %s\n", pathname,
> BLOB_SIZE,
> -                        strerror(errno));
> -            } else {
> -                size_t i;
> -
> -                for (i = 0; i < BLOB_SIZE; ++i) {
> -                    ((uint8_t *)buf)[i] = i;
> -                }
> -                munmap(buf, BLOB_SIZE);
> -                ret = 0;
> -            }
> -        }
> -        close(fd);
> -        if (ret == -1) {
> -            unlink(pathname);
> -            g_free(pathname);
> -        }
> +    g_assert_no_error(error);
> +    close(fd);
> +
> +    for (i = 0; i < BLOB_SIZE; i++) {
> +        buf[i] = i;
>      }
>
> -    return ret == -1 ? NULL : pathname;
> +    error = NULL;
>

Not necessary either.


> +    g_file_set_contents(pathname, (char *)buf, BLOB_SIZE, &error);
> +    g_assert_no_error(error);
> +
> +    return pathname;
>  }
>
>  static void test_i440fx_firmware(FirmwareTestFixture *fixture,
> @@ -398,8 +377,6 @@ static void request_pflash(FirmwareTestFixture
> *fixture,
>      fixture->is_bios = false;
>  }
>
> -#endif /* _WIN32 */
> -
>  int main(int argc, char **argv)
>  {
>      TestData data;
> @@ -410,10 +387,8 @@ int main(int argc, char **argv)
>
>      qtest_add_data_func("i440fx/defaults", &data, test_i440fx_defaults);
>      qtest_add_data_func("i440fx/pam", &data, test_i440fx_pam);
> -#ifndef _WIN32
>      add_firmware_test("i440fx/firmware/bios", request_bios);
>      add_firmware_test("i440fx/firmware/pflash", request_pflash);
> -#endif
>
>      return g_test_run();
>  }
> --
> 2.34.1
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 5531 bytes --]