Re: [PATCH] semihosting: add O_BINARY flag in host_open for NT compatibility

[Evgeny Iakovlev]

On 1/6/2023 10:48, Bin Meng wrote:
> On Fri, Jan 6, 2023 at 3:39 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> > On 5/1/23 22:19, Evgeny Iakovlev wrote:
> > > Windows open(2) implementations opens files in text mode by default and
> > > needs a Windows-only O_BINARY flag to open files as binary. Qemu already
> > s/Qemu/QEMU/
> >
> > > knows about that flag in osdep.h, so we can just add it to the
> > > host_flags for better compatibility when running qemu on Windows.
> > s/qemu/QEMU/
> >
> > > Signed-off-by: Evgeny Iakovlev <eiakovlev@linux.microsoft.com>
> > > ---
> > >    semihosting/syscalls.c | 2 ++
> > >    1 file changed, 2 insertions(+)
> > >
> > > diff --git a/semihosting/syscalls.c b/semihosting/syscalls.c
> > > index 508a0ad88c..00f77507e5 100644
> > > --- a/semihosting/syscalls.c
> > > +++ b/semihosting/syscalls.c
> > > @@ -278,6 +278,8 @@ static void host_open(CPUState *cs, gdb_syscall_complete_cb 
> > > complete,
> > >            host_flags |= O_EXCL;
> > >        }
> > >
> > > +    host_flags |= O_BINARY;
> > > +
> > >        ret = open(p, host_flags, mode);
> > >        if (ret < 0) {
> > >            complete(cs, -1, errno);
> > Alternatively with more churn:
> >
> > -- >8 --
> > diff --git a/semihosting/syscalls.c b/semihosting/syscalls.c
> > index 508a0ad88c..b621d78c2d 100644
> > --- a/semihosting/syscalls.c
> > +++ b/semihosting/syscalls.c
> > @@ -253,7 +253,7 @@ static void host_open(CPUState *cs,
> > gdb_syscall_complete_cb complete,
> >    {
> >        CPUArchState *env G_GNUC_UNUSED = cs->env_ptr;
> >        char *p;
> > -    int ret, host_flags;
> > +    int ret, host_flags = O_BINARY;
> >
> >        ret = validate_lock_user_string(&p, cs, fname, fname_len);
> >        if (ret < 0) {
> > @@ -262,11 +262,11 @@ static void host_open(CPUState *cs,
> > gdb_syscall_complete_cb complete,
> >        }
> >
> >        if (gdb_flags & GDB_O_WRONLY) {
> > -        host_flags = O_WRONLY;
> > +        host_flags |= O_WRONLY;
> >        } else if (gdb_flags & GDB_O_RDWR) {
> > -        host_flags = O_RDWR;
> > +        host_flags |= O_RDWR;
> >        } else {
> > -        host_flags = O_RDONLY;
> > +        host_flags |= O_RDONLY;
> >        }
> >        if (gdb_flags & GDB_O_CREAT) {
> >            host_flags |= O_CREAT;
> > ---
> >
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> With Philippe's comments addressed,
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Done, sent out a v2, thanks!