All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: eiakovlev@linux.microsoft.com
Cc: Peter Maydell <peter.maydell@linaro.org>,
	qemu-devel@nongnu.org, bmeng.cn@gmail.com, philmd@linaro.org
Subject: Re: [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility
Date: Mon, 16 Jan 2023 16:25:41 +0000	[thread overview]
Message-ID: <87fscae97o.fsf@linaro.org> (raw)
In-Reply-To: <1e5c8643-e756-9110-70f1-a83e301cca03@linux.microsoft.com>


eiakovlev@linux.microsoft.com writes:

> On 1/6/23 7:58 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On Fri, 6 Jan 2023 at 18:22, Evgeny Iakovlev
>> <eiakovlev@linux.microsoft.com> wrote:
>> >
>> >
>> > On 1/6/2023 17:28, Peter Maydell wrote:
>> >> On Fri, 6 Jan 2023 at 15:44, Alex Bennée <alex.bennee@linaro.org> wrote:
>> >>> Peter Maydell <peter.maydell@linaro.org> writes:
>> >> I think the theory when the semihosting API was originally designed
>> >> decades ago was basically "when the guest does fopen(...) this
>> >> should act like it does on the host". So as a bit of portable
>> >> guest code you would say whether you wanted a binary or a text
>> >> file, and the effect would be that if you were running on Windows
>> >> and you output a text file then you'd get \r\n like the user
>> >> probably expected, and if on Linux you get \n.
>>  > If SYS_OPEN is supposed to call fopen (i didn't actually know
>> that..)
>> > then it does make more sense for binary/text mode to be propagated from
>> > guest.
>> It's not required to literally call fopen(). It just has to
>> give the specified semantics for when the guest passes it a
>> mode integer, which is defined in terms of the ISO C
>> fopen() string semantics for "r", "rb", "r+", "r+b", etc.
>>  > Qemu's implementation calls open(2) though, which is not correct
>> > at all then. Well, as long as qemu does that, there is no
>> > posix-compliant way to tell open(2) if it should use binary or text
>> > mode, there is no notion of that as far as posix (and most
>> > implementations) is concerned.
>> QEMU doesn't have to be pure POSIX compliant: we know what our
>> supported host platforms are and we can freely use extensions
>> they provide. If we want to achieve the semantics that semihosting
>> asks for then we can do that with open(), by passing O_BINARY when
>> the mode integer from the guest corresponds to a string with "b" in it.
>> I'm about 50:50 on whether we should do that vs documenting and
>> commenting that we deliberately produce the same behaviour on all
>> platforms by ignoring the 'b' flag, though.
>> thanks
>> -- PMM
>> 
>
> Thanks Peter, i think i see your point. However, if you ask me, i feel
> like advertising a feature to guest code and only implementing it on 1
> platform that supports it just because it has a non-standard POSIX
> implementation will only confuse the issue further.
> Guest code doesn't want to care whether or not an emulator is running
> on Linux or Windows, there is no notion of that leaking to guest code.
> What it cares about is being able to consistently use a certain
> feature in their code.
> So i think it would be rather useless to implement it on Windows-only
> given there is a clear alternative to switch to fopen. Just my 2
> cents.

It's not switching to fopen() that is the issue - it's the interaction
with gdb (via gdbstub) which has no idea about the distinction. Anyway I
already have the patch queued with an additional note in the
documentation that all file accesses are in binary mode.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


  reply	other threads:[~2023-01-16 16:27 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-06 10:20 [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility Evgeny Iakovlev
2023-01-06 13:51 ` Alex Bennée
2023-01-06 14:13 ` Peter Maydell
2023-01-06 15:33   ` Alex Bennée
2023-01-06 16:28     ` Peter Maydell
2023-01-06 18:22       ` Evgeny Iakovlev
2023-01-06 18:58         ` Peter Maydell
2023-01-16 15:56           ` eiakovlev
2023-01-16 16:25             ` Alex Bennée [this message]
2023-01-16 16:39             ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87fscae97o.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=bmeng.cn@gmail.com \
    --cc=eiakovlev@linux.microsoft.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.