poke-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] [IOS] Avoid spurious EOF when writing partial bytes


From: Jose E. Marchesi
Subject: [PATCH] [IOS] Avoid spurious EOF when writing partial bytes
Date: Sat, 27 Mar 2021 09:56:10 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux)

Hi Egeyar, all.

While working on other bugs we found an issue with the IOS.  We are
proposing the following patch.

Generally speaking, an IO space can be made to grow by mapping (writing)
data past the end.

Lets create an empty file:

  (poke) .file/c lalala
  (poke) dump
  76543210  0011 2233 4455 6677 8899 aabb ccdd eeff  0123456789ABCDEF
  00000000:  
  (poke) iosize/#B
  0UL

Then we write past the end:

  (poke) byte @ 3#B = 0xff
  (poke) dump
  76543210  0011 2233 4455 6677 8899 aabb ccdd eeff  0123456789ABCDEF
  00000000: 0000 00ff                                ....
  (poke) iosize/#B
  4UL

This is the expected behavior.

However, consider what happens when you write weird integers past the
end of the IO space, like in:

(poke) uint<3> @ iosize = 2

In this case, the IO subsystem has to read a byte in order to complete
the contents of the partially written byte.  Currently this results in
an EOF exception being raised by ios_write_u?int.  No good.

In the patch attached:

- We change the macro IOS_GET_C_ERR_CHCK in order to return a zero
  completing byte instead of returning IOS_EIOFF.  This relies in the
  fact the macro is only used in ios_write_int_common, not in
  ios_read_int_common.

- We add a new test for this.

- We rename the test maps-strings-diag-4.pk to maps-strings-4.pk, and
  adjust it accordingly.

The last point is due to the fact that we are changing semantics here.
Up to now, trying to write a value _partially_ past the end of the IO
space resulted in an error (EIOFF, invalid offset):

      |     IO space     |
                      < value >

Now, this just grows the IO space, for any kind of value: integers,
strings, etc.

Does this look good to you?

commit 867d7ff1641264b560f0b0fad6ab0931cf8ab204
Author: Jose E. Marchesi <jose.marchesi@oracle.com>
Date:   Sat Mar 27 09:53:13 2021 +0100

    ios: avoid spurious EOF when writing weird integers past the end of an IOS
    
    2021-03-27  Jose E. Marchesi  <jemarch@gnu.org>
    
            * testsuite/poke.map/ass-map-20.pk: New test.
            * testsuite/poke.map/maps-strings-4.pk: Renamed from
            maps-strings-diag-4.pk.
            * testsuite/Makefile.am (EXTRA_DIST): Update accordingly.

diff --git a/ChangeLog b/ChangeLog
index 5b52061e..5453b2bf 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2021-03-27  Jose E. Marchesi  <jemarch@gnu.org>
+
+       * testsuite/poke.map/ass-map-20.pk: New test.
+       * testsuite/poke.map/maps-strings-4.pk: Renamed from
+       maps-strings-diag-4.pk.
+       * testsuite/Makefile.am (EXTRA_DIST): Update accordingly.
+
 2021-03-27  Jose E. Marchesi  <jemarch@gnu.org>
            Mohammad-Reza Nabipoor  <m.nabipoor@yahoo.com>
 
diff --git a/libpoke/ios.c b/libpoke/ios.c
index 75ea05de..3e04c730 100644
--- a/libpoke/ios.c
+++ b/libpoke/ios.c
@@ -32,13 +32,16 @@
 #include "ios.h"
 #include "ios-dev.h"
 
-#define IOS_GET_C_ERR_CHCK(c, io, off)                                \
-  {                                                                \
+#define IOS_GET_C_ERR_CHCK(c, io, off)                                 \
+  {                                                                    \
     uint8_t ch;                                                        \
-    int ret = (io)->dev_if->pread ((io)->dev, &ch, 1, off);         \
-    if (ret == IOD_EOF)                                                \
-      return IOS_EIOFF;                                                \
-    (c) = ch;                                                        \
+    int ret = (io)->dev_if->pread ((io)->dev, &ch, 1, off);            \
+    /* If the pread reports an EOF, it means the partial byte */       \
+    /* we are writing is past the end of the IOS.  That may or */      \
+    /* may not be supported in the underlying IOD, but in any */       \
+    /* case that will be resolved when the completed byte is */        \
+    /* written back.  */                                               \
+    (c) = ret == IOD_EOF ? 0 : ch;                                     \
   }
 
 #define IOS_PUT_C_ERR_CHCK(c, io, len, off)                \
diff --git a/testsuite/Makefile.am b/testsuite/Makefile.am
index 28968938..73b3bef4 100644
--- a/testsuite/Makefile.am
+++ b/testsuite/Makefile.am
@@ -124,6 +124,7 @@ EXTRA_DIST = \
   poke.map/ass-map-17.pk \
   poke.map/ass-map-18.pk \
   poke.map/ass-map-19.pk \
+  poke.map/ass-map-20.pk \
   poke.map/ass-map-struct-int-1.pk \
   poke.map/func-map-1.pk \
   poke.map/func-map-2.pk \
@@ -276,9 +277,9 @@ EXTRA_DIST = \
   poke.map/maps-strings-1.pk \
   poke.map/maps-strings-2.pk \
   poke.map/maps-strings-3.pk \
+  poke.map/maps-strings-4.pk \
   poke.map/maps-strings-diag-1.pk \
   poke.map/maps-strings-diag-2.pk \
-  poke.map/maps-strings-diag-4.pk \
   poke.map/maps-structs-1.pk \
   poke.map/maps-structs-2.pk \
   poke.map/maps-structs-3.pk \
diff --git a/testsuite/poke.map/ass-map-20.pk b/testsuite/poke.map/ass-map-20.pk
new file mode 100644
index 00000000..5fb97e92
--- /dev/null
+++ b/testsuite/poke.map/ass-map-20.pk
@@ -0,0 +1,7 @@
+/* { dg-do run } */
+
+/* { dg-command {.mem foo} } */
+/* { dg-command {var is = iosize} } */
+/* { dg-command {uint<3> @ is = 2} } */
+/* { dg-command {uint<3> @ is} } */
+/* { dg-output ".uint<3>. 2" } */
diff --git a/testsuite/poke.map/maps-strings-4.pk 
b/testsuite/poke.map/maps-strings-4.pk
new file mode 100644
index 00000000..3ced1d4c
--- /dev/null
+++ b/testsuite/poke.map/maps-strings-4.pk
@@ -0,0 +1,11 @@
+/* { dg-do run } */
+/* { dg-data {c*} {0x66 0x6f 0x6f 0x00  0x50 0x60 0x70 0x80   0x90 0xa0 0xb0 
0xc0} } */
+
+/* Writing a string partially past the end of the IO space is allowed.
+   The IO space grows.  */
+
+/* { dg-command { string @ 8#B + 1#b = "quux" } } */
+/* { dg-command { iosize/#B } } */
+/* { dg-output "13UL" } */
+/* { dg-command { string @ 8#B + 1#b } } */
+/* { dg-output "\n\"quux\"" } */
diff --git a/testsuite/poke.map/maps-strings-diag-4.pk 
b/testsuite/poke.map/maps-strings-diag-4.pk
deleted file mode 100644
index 5d7943d6..00000000
--- a/testsuite/poke.map/maps-strings-diag-4.pk
+++ /dev/null
@@ -1,5 +0,0 @@
-/* { dg-do run } */
-/* { dg-data {c*} {0x66 0x6f 0x6f 0x00  0x50 0x60 0x70 0x80   0x90 0xa0 0xb0 
0xc0} } */
-
-/* { dg-command { try string @ (8*8+1)#b = "quux"; catch if E_eof { printf 
"caught\n"; } } } */
-/* { dg-output "caught" } */



reply via email to

[Prev in Thread] Current Thread [Next in Thread]