[PATCH 38/51] tests/qtest: {ahci,ide}-test: Open file in binary mode

Bin Meng posted 51 patches 3 years, 5 months ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Thomas Huth <thuth@redhat.com>, Wainer dos Santos Moschetta <wainersm@redhat.com>, Beraldo Leal <bleal@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Stefan Berger <stefanb@linux.vnet.ibm.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Greg Kurz <groug@kaod.org>, Christian Schoenebeck <qemu_oss@crudebyte.com>, "Cédric Le Goater" <clg@kaod.org>, Daniel Henrique Barboza <danielhb413@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, Gerd Hoffmann <kraxel@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Yanan Wang <wangyanan55@huawei.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Michael Roth <michael.roth@amd.com>, Konstantin Kostiuk <kkostiuk@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Juan Quintela <quintela@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, John Snow <jsnow@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Andrew Jeffery <andrew@aj.id.au>, Joel Stanley <joel@jms.id.au>, "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <ani@anisinha.ca>, Alexander Bulekov <alxndr@bu.edu>, Bandan Das <bsd@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Darren Kenny <darren.kenny@oracle.com>, Qiuhao Li <Qiuhao.Li@outlook.com>, Havard Skinnemoen <hskinnemoen@google.com>, Tyrone Ting <kfting@nuvoton.com>, Markus Armbruster <armbru@redhat.com>, Coiby Xu <Coiby.Xu@gmail.com>, Jason Wang <jasowang@redhat.com>, Fam Zheng <fam@euphon.net>
There is a newer version of this series
[PATCH 38/51] tests/qtest: {ahci,ide}-test: Open file in binary mode
Posted by Bin Meng 3 years, 5 months ago
From: Xuzhou Cheng <xuzhou.cheng@windriver.com>

By default Windows opens file in text mode, while a POSIX compliant
implementation treats text files and binary files the same.

The fopen() 'mode' string can include the letter 'b' to indicate
binary mode shall be used. POSIX spec says the character 'b' shall
have no effect, but is allowed for ISO C standard conformance.
Let's add the letter 'b' which works on both POSIX and Windows.

Similar situation applies to the open() 'flags' where O_BINARY is
used for binary mode.

Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

 tests/qtest/ahci-test.c | 2 +-
 tests/qtest/ide-test.c  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
index bce9ff770c..be11508c75 100644
--- a/tests/qtest/ahci-test.c
+++ b/tests/qtest/ahci-test.c
@@ -1453,7 +1453,7 @@ static int prepare_iso(size_t size, unsigned char **buf, char **name)
      * Close the file and reopen it.
      */
     close(fd);
-    fd = open(cdrom_path, O_WRONLY);
+    fd = open(cdrom_path, O_WRONLY | O_BINARY);
     g_assert(fd != -1);
 #endif
 
diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
index c5cad6c0be..ee03dea4fa 100644
--- a/tests/qtest/ide-test.c
+++ b/tests/qtest/ide-test.c
@@ -892,7 +892,7 @@ static void cdrom_pio_impl(int nblocks)
 
     /* Prepopulate the CDROM with an interesting pattern */
     generate_pattern(pattern, patt_len, ATAPI_BLOCK_SIZE);
-    fh = fopen(tmp_path, "w+");
+    fh = fopen(tmp_path, "wb+");
     ret = fwrite(pattern, ATAPI_BLOCK_SIZE, patt_blocks, fh);
     g_assert_cmpint(ret, ==, patt_blocks);
     fclose(fh);
@@ -993,7 +993,7 @@ static void test_cdrom_dma(void)
     prdt[0].size = cpu_to_le32(len | PRDT_EOT);
 
     generate_pattern(pattern, ATAPI_BLOCK_SIZE * 16, ATAPI_BLOCK_SIZE);
-    fh = fopen(tmp_path, "w+");
+    fh = fopen(tmp_path, "wb+");
     ret = fwrite(pattern, ATAPI_BLOCK_SIZE, 16, fh);
     g_assert_cmpint(ret, ==, 16);
     fclose(fh);
-- 
2.34.1
Re: [PATCH 38/51] tests/qtest: {ahci, ide}-test: Open file in binary mode
Posted by Marc-André Lureau 3 years, 5 months ago
Hi

On Wed, Aug 24, 2022 at 3:08 PM Bin Meng <bmeng.cn@gmail.com> wrote:

> From: Xuzhou Cheng <xuzhou.cheng@windriver.com>
>
> By default Windows opens file in text mode, while a POSIX compliant
> implementation treats text files and binary files the same.
>
> The fopen() 'mode' string can include the letter 'b' to indicate
> binary mode shall be used. POSIX spec says the character 'b' shall
> have no effect, but is allowed for ISO C standard conformance.
> Let's add the letter 'b' which works on both POSIX and Windows.
>
> Similar situation applies to the open() 'flags' where O_BINARY is
> used for binary mode.
>
> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
>  tests/qtest/ahci-test.c | 2 +-
>  tests/qtest/ide-test.c  | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
> index bce9ff770c..be11508c75 100644
> --- a/tests/qtest/ahci-test.c
> +++ b/tests/qtest/ahci-test.c
> @@ -1453,7 +1453,7 @@ static int prepare_iso(size_t size, unsigned char
> **buf, char **name)
>       * Close the file and reopen it.
>       */
>      close(fd);
> -    fd = open(cdrom_path, O_WRONLY);
> +    fd = open(cdrom_path, O_WRONLY | O_BINARY);
>      g_assert(fd != -1);
>

that should be gone in next iteration, with g_mkstemp() usage.


>  #endif
>
> diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
> index c5cad6c0be..ee03dea4fa 100644
> --- a/tests/qtest/ide-test.c
> +++ b/tests/qtest/ide-test.c
> @@ -892,7 +892,7 @@ static void cdrom_pio_impl(int nblocks)
>
>      /* Prepopulate the CDROM with an interesting pattern */
>      generate_pattern(pattern, patt_len, ATAPI_BLOCK_SIZE);
> -    fh = fopen(tmp_path, "w+");
> +    fh = fopen(tmp_path, "wb+");
>      ret = fwrite(pattern, ATAPI_BLOCK_SIZE, patt_blocks, fh);
>      g_assert_cmpint(ret, ==, patt_blocks);
>      fclose(fh);
> @@ -993,7 +993,7 @@ static void test_cdrom_dma(void)
>      prdt[0].size = cpu_to_le32(len | PRDT_EOT);
>
>      generate_pattern(pattern, ATAPI_BLOCK_SIZE * 16, ATAPI_BLOCK_SIZE);
> -    fh = fopen(tmp_path, "w+");
> +    fh = fopen(tmp_path, "wb+");
>      ret = fwrite(pattern, ATAPI_BLOCK_SIZE, 16, fh);
>      g_assert_cmpint(ret, ==, 16);
>      fclose(fh);
> --
> 2.34.1
>
>
>
ack this part,
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


-- 
Marc-André Lureau
Re: [PATCH 38/51] tests/qtest: {ahci,ide}-test: Open file in binary mode
Posted by Thomas Huth 3 years, 5 months ago
On 24/08/2022 11.40, Bin Meng wrote:
> From: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> 
> By default Windows opens file in text mode, while a POSIX compliant
> implementation treats text files and binary files the same.
> 
> The fopen() 'mode' string can include the letter 'b' to indicate
> binary mode shall be used. POSIX spec says the character 'b' shall
> have no effect, but is allowed for ISO C standard conformance.
> Let's add the letter 'b' which works on both POSIX and Windows.
> 
> Similar situation applies to the open() 'flags' where O_BINARY is
> used for binary mode.
> 
> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
> 
>   tests/qtest/ahci-test.c | 2 +-
>   tests/qtest/ide-test.c  | 4 ++--
>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
> index bce9ff770c..be11508c75 100644
> --- a/tests/qtest/ahci-test.c
> +++ b/tests/qtest/ahci-test.c
> @@ -1453,7 +1453,7 @@ static int prepare_iso(size_t size, unsigned char **buf, char **name)
>        * Close the file and reopen it.
>        */
>       close(fd);
> -    fd = open(cdrom_path, O_WRONLY);
> +    fd = open(cdrom_path, O_WRONLY | O_BINARY);
>       g_assert(fd != -1);
>   #endif

Could you please squash this hunk into patch 32/51 where you introduced this 
code?

  Thomas