[PATCH] RISC-V: fix setting up std streams in init_semihosting()

[PATCH] RISC-V: fix setting up std streams in init_semihosting()

[Ram Nalamothu (QUIC)]

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

Without this patch, the following simple test would fail to run as expected.

#include <stdio.h>
int main() {
  int n;
  printf("Enter a number\n");
  scanf("%d", &n);
  printf("Entered number: %d\n", n);
  return 0;
}

If the patch looks good, could someone please commit it on behalf of me as I don't have commit rights.

-- 8< --

Currently init_semihosting() assumes the return value from _open()
call as the file descriptor handle and that is incorrect.

The semihost _open() call returns the fdtable index returned by the
__add_fdentry() for the file opened.

[-- Attachment #2: 0001-RISC-V-fix-setting-up-std-streams-in-init_semihostin.patch --]
[-- Type: application/octet-stream, Size: 2051 bytes --]

From 966aa280a1c51759ff8ab8e6cdda225a2093f17b Mon Sep 17 00:00:00 2001
From: Venkata Ramanaiah Nalamothu <quic_vnalamot@quicinc.com>
Date: Wed, 3 Jan 2024 12:22:16 +0530
Subject: [PATCH] RISC-V: fix setting up std streams in init_semihosting()
To: newlib@sourceware.org,
    kito.cheng@sifive.com,
    craig.blackmore@embecosm.com

Currently init_semihosting() assumes the return value from _open()
call as the file descriptor handle and that is incorrect.

The semihost _open() call returns the fdtable index returned by the
__add_fdentry() for the file opened.
---
 libgloss/riscv/semihost-sys_fdtable.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/libgloss/riscv/semihost-sys_fdtable.c b/libgloss/riscv/semihost-sys_fdtable.c
index 152c92d15..d62e3f970 100644
--- a/libgloss/riscv/semihost-sys_fdtable.c
+++ b/libgloss/riscv/semihost-sys_fdtable.c
@@ -24,25 +24,26 @@ static struct fdentry fdtable[RISCV_MAX_OPEN_FILES];
 void __attribute__ ((constructor))
 init_semihosting ()
 {
-  int handle;
+  int i;
 
-  for (int i=0; i<RISCV_MAX_OPEN_FILES; i++)
+  for (i=0; i<RISCV_MAX_OPEN_FILES; i++)
     fdtable[i].handle = -1;
 
-  /* Set up std streams.  */
+  /* Set up std streams. Note that the semihost _open() call returns an index
+     into the fdtable.  */
   /* stdin.  */
-  handle = _open (":tt", O_RDONLY);
-  fdtable[STDIN_FILENO].handle = handle;
+  i = _open (":tt", O_RDONLY);
+  fdtable[STDIN_FILENO].handle = fdtable[i].handle;
   fdtable[STDIN_FILENO].pos = 0;
 
   /* stdout.  */
-  handle = _open (":tt", O_WRONLY|O_CREAT|O_TRUNC);
-  fdtable[STDOUT_FILENO].handle = handle;
+  i = _open (":tt", O_WRONLY|O_CREAT|O_TRUNC);
+  fdtable[STDOUT_FILENO].handle = fdtable[i].handle;
   fdtable[STDOUT_FILENO].pos = 0;
 
   /* stderr.  */
-  handle = _open (":tt", O_WRONLY|O_CREAT|O_APPEND);
-  fdtable[STDERR_FILENO].handle = handle;
+  i = _open (":tt", O_WRONLY|O_CREAT|O_APPEND);
+  fdtable[STDERR_FILENO].handle = fdtable[i].handle;
   fdtable[STDERR_FILENO].pos = 0;
 }
 
-- 
2.17.1

Re: [PATCH] RISC-V: fix setting up std streams in init_semihosting()

[Kito Cheng]

The change seems weird to me at my first time reading, however it
makes sense after reading _open and __add_fdentry.

So LGTM, thanks for fixing this issue :)

(BTW, I don't have commit right either :P)

On Wed, Jan 3, 2024 at 4:03 PM Ram Nalamothu (QUIC)
<quic_vnalamot@quicinc.com> wrote:
>
> Without this patch, the following simple test would fail to run as expected.
>
> #include <stdio.h>
> int main() {
>   int n;
>   printf("Enter a number\n");
>   scanf("%d", &n);
>   printf("Entered number: %d\n", n);
>   return 0;
> }
>
> If the patch looks good, could someone please commit it on behalf of me as I don't have commit rights.
>
> -- 8< --
>
> Currently init_semihosting() assumes the return value from _open()
> call as the file descriptor handle and that is incorrect.
>
> The semihost _open() call returns the fdtable index returned by the
> __add_fdentry() for the file opened.

RE: [PATCH] RISC-V: fix setting up std streams in init_semihosting()

[Ram Nalamothu (QUIC)]

Ping

-----Original Message-----
From: Kito Cheng <kito.cheng@sifive.com> 
Sent: Wednesday, January 3, 2024 1:48 PM
To: Ram Nalamothu (QUIC) <quic_vnalamot@quicinc.com>
Cc: newlib@sourceware.org; craig.blackmore@embecosm.com
Subject: Re: [PATCH] RISC-V: fix setting up std streams in init_semihosting()

WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.

The change seems weird to me at my first time reading, however it makes sense after reading _open and __add_fdentry.

So LGTM, thanks for fixing this issue :)

(BTW, I don't have commit right either :P)

On Wed, Jan 3, 2024 at 4:03 PM Ram Nalamothu (QUIC) <quic_vnalamot@quicinc.com> wrote:
>
> Without this patch, the following simple test would fail to run as expected.
>
> #include <stdio.h>
> int main() {
>   int n;
>   printf("Enter a number\n");
>   scanf("%d", &n);
>   printf("Entered number: %d\n", n);
>   return 0;
> }
>
> If the patch looks good, could someone please commit it on behalf of me as I don't have commit rights.
>
> -- 8< --
>
> Currently init_semihosting() assumes the return value from _open() 
> call as the file descriptor handle and that is incorrect.
>
> The semihost _open() call returns the fdtable index returned by the
> __add_fdentry() for the file opened.

RE: [PATCH] RISC-V: fix setting up std streams in init_semihosting()

[Ram Nalamothu (QUIC)]

Hi,

What is missing for this patch to be pushed?
If nothing, could someone please help me by applying this patch.

Regards,
Ram

-----Original Message-----
From: Ram Nalamothu (QUIC) <quic_vnalamot@quicinc.com> 
Sent: Friday, January 5, 2024 5:22 PM
To: newlib@sourceware.org
Cc: craig.blackmore@embecosm.com; kito.cheng@sifive.com
Subject: RE: [PATCH] RISC-V: fix setting up std streams in init_semihosting()

Ping

-----Original Message-----
From: Kito Cheng <kito.cheng@sifive.com>
Sent: Wednesday, January 3, 2024 1:48 PM
To: Ram Nalamothu (QUIC) <quic_vnalamot@quicinc.com>
Cc: newlib@sourceware.org; craig.blackmore@embecosm.com
Subject: Re: [PATCH] RISC-V: fix setting up std streams in init_semihosting()

WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.

The change seems weird to me at my first time reading, however it makes sense after reading _open and __add_fdentry.

So LGTM, thanks for fixing this issue :)

(BTW, I don't have commit right either :P)

On Wed, Jan 3, 2024 at 4:03 PM Ram Nalamothu (QUIC) <quic_vnalamot@quicinc.com> wrote:
>
> Without this patch, the following simple test would fail to run as expected.
>
> #include <stdio.h>
> int main() {
>   int n;
>   printf("Enter a number\n");
>   scanf("%d", &n);
>   printf("Entered number: %d\n", n);
>   return 0;
> }
>
> If the patch looks good, could someone please commit it on behalf of me as I don't have commit rights.
>
> -- 8< --
>
> Currently init_semihosting() assumes the return value from _open() 
> call as the file descriptor handle and that is incorrect.
>
> The semihost _open() call returns the fdtable index returned by the
> __add_fdentry() for the file opened.

Re: [PATCH] RISC-V: fix setting up std streams in init_semihosting()

[Corinna Vinschen]

On Jan 10 07:57, Ram Nalamothu (QUIC) wrote:
> Hi,
> 
> What is missing for this patch to be pushed?
> If nothing, could someone please help me by applying this patch.

Pushed.

Thanks,
Corinna

> 
> Regards,
> Ram
> 
> -----Original Message-----
> From: Ram Nalamothu (QUIC) <quic_vnalamot@quicinc.com> 
> Sent: Friday, January 5, 2024 5:22 PM
> To: newlib@sourceware.org
> Cc: craig.blackmore@embecosm.com; kito.cheng@sifive.com
> Subject: RE: [PATCH] RISC-V: fix setting up std streams in init_semihosting()
> 
> Ping
> 
> -----Original Message-----
> From: Kito Cheng <kito.cheng@sifive.com>
> Sent: Wednesday, January 3, 2024 1:48 PM
> To: Ram Nalamothu (QUIC) <quic_vnalamot@quicinc.com>
> Cc: newlib@sourceware.org; craig.blackmore@embecosm.com
> Subject: Re: [PATCH] RISC-V: fix setting up std streams in init_semihosting()
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.
> 
> The change seems weird to me at my first time reading, however it makes sense after reading _open and __add_fdentry.
> 
> So LGTM, thanks for fixing this issue :)
> 
> (BTW, I don't have commit right either :P)
> 
> On Wed, Jan 3, 2024 at 4:03 PM Ram Nalamothu (QUIC) <quic_vnalamot@quicinc.com> wrote:
> >
> > Without this patch, the following simple test would fail to run as expected.
> >
> > #include <stdio.h>
> > int main() {
> >   int n;
> >   printf("Enter a number\n");
> >   scanf("%d", &n);
> >   printf("Entered number: %d\n", n);
> >   return 0;
> > }
> >
> > If the patch looks good, could someone please commit it on behalf of me as I don't have commit rights.
> >
> > -- 8< --
> >
> > Currently init_semihosting() assumes the return value from _open() 
> > call as the file descriptor handle and that is incorrect.
> >
> > The semihost _open() call returns the fdtable index returned by the
> > __add_fdentry() for the file opened.