Re: [Minios-devel] [PATCH 2/2] lib/vfscore: Add both the block and echo support

[Vlad-Andrei BĂDOIU (78692) <vlad_andrei.badoiu@xxxxxxxxxxxxxxx>]

Hey Yuri,

Thank you for the review. You're right, this implementation reads
only 1 character from stdin and it should actually read up to
count or until \n is read. (We don't seem to have EOF defined)

I've modified the code such that the busy waiting is done until we've
read count bytes or the character \n has been read. Furthermore, for kvm
the character \r is returned so I replaced it with \n. I have tested 
this code on
the micropython shell and it works. Let me know if  this looks all right to
you before I submit the v2 patch.

@@ -37,29 +37,34 @@
  #include <uk/plat/console.h>
  #include <uk/essentials.h>

+/* One function for stderr and stdout */
+static ssize_t stdout_write(struct vfscore_file *vfscore_file __unused,
+                           const void *buf, size_t count)
+{
+       return ukplat_coutk(buf, count);
+}
+
  static ssize_t stdin_read(struct vfscore_file *vfscore_file __unused,
                           void *buf, size_t count)
  {
-       int read_count;
+       int bytes_read, byes_total = 0;

-       read_count = ukplat_cink(buf, count);
+       do {
+               bytes_read = ukplat_cink(buf + byes_total, count - 
byes_total);
+               if (bytes_read <= 0)
+                       continue;
+               if (((char *)buf)[byes_total] == '\r')
+                       ((char *)buf)[byes_total] = '\n';

-#ifdef CONFIG_UK_STDIN_BLOCKING
-       while (read_count <= 0)
-               read_count = ukplat_cink(buf, count);
-#endif
+               stdout_write(vfscore_file, ((char *)buf + byes_total),
+                                               bytes_read);
+               byes_total += bytes_read;

-#ifdef CONFIG_UK_STDIN_ECHO
-       stdout_write(vfscore_file, buf, read_count, 0);
-#endif
-       return read_count;
-}
+       } while (byes_total == 0 || bytes_read <= 0 || (byes_total < 
count &&
+                       ((char *)buf)[byes_total - bytes_read] != '\n'));

-/* One function for stderr and stdout */
-static ssize_t stdout_write(struct vfscore_file *vfscore_file __unused,
-                           const void *buf, size_t count)
-{

-       return ukplat_coutk(buf, count);
+
+       return byes_total;
  }

  static struct vfscore_fops stdin_fops = {


On 12/3/18 5:24 PM, Yuri Volchkov wrote:
> Neither CONFIG_UK_STDIN_BLOCKING, nor CONFIG_UK_STDIN_ECHO is not
> present in any configurations. And I don't think they should. Isn't the
> code under these ifdefs implements a bit more correct behavior of the
> read function? Even though it would not build if I manually enable these
> config.
>
> I said "more correct" because this implementation reads only 1
> character from stdin. I think it is more reasonable to mimic the
> whatever normal read does (read until EOF or \n met).
>
> That is a step in the right direction I think, but a little bit more
> needed to be done.
>
> "Vlad-Andrei BĂDOIU (78692)" <vlad_andrei.badoiu@xxxxxxxxxxxxxxx>
> writes:
>
>> The block behaviour is needed by the python shell. Currently
>> uk_cink return 0 if no character is being inputted and python
>> uses fgets on stdin which causes the console to close
>> immediately. The echo behaviour is needed for feedback on the
>> typed input. Both features are guarded by defines (CONFIG_UK_
>> STDIN_BLOCKING and CONFIG_UK_STDIN_ECHO).
>>
>> Signed-off-by: Vlad-Andrei Badoiu <vlad_andrei.badoiu@xxxxxxxxxxxxxxx>
>> ---
>>   lib/vfscore/stdio.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/lib/vfscore/stdio.c b/lib/vfscore/stdio.c
>> index 6acbb32..ec61e3a 100644
>> --- a/lib/vfscore/stdio.c
>> +++ b/lib/vfscore/stdio.c
>> @@ -44,6 +44,14 @@ static ssize_t stdin_read(struct vfscore_file 
>> *vfscore_file __unused,
>>   
>>      read_count = ukplat_cink(buf, count);
>>   
>> +#ifdef CONFIG_UK_STDIN_BLOCKING
>> +    while (read_count <= 0)
>> +            read_count = ukplat_cink(buf, count);
>> +#endif
>> +
>> +#ifdef CONFIG_UK_STDIN_ECHO
>> +    stdout_write(vfscore_file, buf, read_count, 0);
>> +#endif
>>      return read_count;
>>   }
>>   
>> -- 
>> 2.19.1
>>
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel