Improve UEFI stdio by Ayush1325 · Pull Request #117174 · rust-lang/rust

Ayush1325 · 2023-10-25T16:21:22Z

Fixed some things suggested in last PR: #116207

cc @dvdhrm
cc @nicholasbishop

rustbot · 2023-10-25T16:21:29Z

r? @joshtriplett

(rustbot has picked a reviewer for you, use r? to override)

library/std/src/sys/uefi/stdio.rs

workingjubilee · 2023-10-30T10:36:16Z

r? workingjubilee

library/std/src/sys/uefi/stdio.rs

workingjubilee · 2023-10-30T22:27:51Z

simple_text_input_read seems a little on the verbose side, but between that and it being named read while used in an implementation of Read::read, yeah, I'll take it.

library/std/src/sys/uefi/stdio.rs

Ayush1325 · 2023-10-31T05:42:30Z

@dvdhrm @workingjubilee Do you think read should ignore non-printable characters instead of returning an error?

workingjubilee · 2023-10-31T07:03:34Z

I think Err in that case is correct: we lost the input, effectively. There's lots of ways to suppress that kind of error if the programmer wants.

dvdhrm

Thanks a lot for improving this! Some comments inline.

I really think we should suppress unknown scancodes. This is what TTYs on linux do, so I think we should follow.

library/std/src/sys/uefi/stdio.rs

workingjubilee · 2023-11-22T21:09:44Z

@rustbot author
There's at least one change we need to settle before we can ship this.

Ayush1325 · 2023-11-30T10:29:02Z

@rustbot ready

library/std/src/sys/uefi/stdio.rs

bors · 2024-01-13T16:45:14Z

☔ The latest upstream changes (presumably #117285) made this pull request unmergeable. Please resolve the merge conflicts.

- Do not drop any character while reading - eabdf == Unsupported status - loop untill read character or error encountered Signed-off-by: Ayush Singh <ayushdevel1325@gmail.com>

Signed-off-by: Ayush Singh <ayushdevel1325@gmail.com>

- is_ebadf always returns false - Allow reading partial characters to buffer - Allow full UTF-16 in stdin Signed-off-by: Ayush Singh <ayushdevel1325@gmail.com>

Amanieu · 2024-02-21T17:32:16Z

ping from triage: @workingjubilee have all your review concerns been addressed?

workingjubilee · 2024-02-22T05:38:45Z

Ah! Yes, it seems so!
@bors r+

bors · 2024-02-22T05:38:48Z

📌 Commit 1fbb00b has been approved by workingjubilee

It is now in the queue for this repository.

bors · 2024-02-22T06:01:27Z

⌛ Testing commit 1fbb00b with merge 026b3b8...

bors · 2024-02-22T08:00:31Z

☀️ Test successful - checks-actions
Approved by: workingjubilee
Pushing 026b3b8 to master...

rust-timer · 2024-02-22T09:14:58Z

Finished benchmarking commit (026b3b8): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.7%	[-0.7%, -0.7%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.5%	[2.1%, 2.9%]	3
Improvements ✅ (primary)	-2.1%	[-2.1%, -2.1%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.1%	[-2.1%, -2.1%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 649.414s -> 648.961s (-0.07%)
Artifact size: 310.96 MiB -> 310.94 MiB (-0.00%)

rustbot assigned joshtriplett Oct 25, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Oct 25, 2023

workingjubilee reviewed Oct 30, 2023

View reviewed changes

library/std/src/sys/uefi/stdio.rs Outdated Show resolved Hide resolved

library/std/src/sys/uefi/stdio.rs Show resolved Hide resolved

rustbot assigned workingjubilee and unassigned joshtriplett Oct 30, 2023

Ayush1325 force-pushed the uefi-stdio-improve branch from 98d0b71 to 6465ab4 Compare October 30, 2023 13:29

dvdhrm suggested changes Oct 30, 2023

View reviewed changes

library/std/src/sys/uefi/stdio.rs Outdated Show resolved Hide resolved

library/std/src/sys/uefi/stdio.rs Outdated Show resolved Hide resolved

Ayush1325 force-pushed the uefi-stdio-improve branch from 6465ab4 to 3c544b3 Compare October 30, 2023 14:46

workingjubilee reviewed Oct 30, 2023

View reviewed changes

library/std/src/sys/uefi/stdio.rs Outdated Show resolved Hide resolved

dvdhrm reviewed Oct 31, 2023

View reviewed changes

library/std/src/sys/uefi/stdio.rs Outdated Show resolved Hide resolved

library/std/src/sys/uefi/stdio.rs Outdated Show resolved Hide resolved

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 22, 2023

Ayush1325 force-pushed the uefi-stdio-improve branch from 3c544b3 to 3c9cae1 Compare November 29, 2023 16:47

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 30, 2023

workingjubilee reviewed Dec 3, 2023

View reviewed changes

library/std/src/sys/uefi/stdio.rs Outdated Show resolved Hide resolved

library/std/src/sys/uefi/stdio.rs Outdated Show resolved Hide resolved

library/std/src/sys/uefi/stdio.rs Outdated Show resolved Hide resolved

Ayush1325 force-pushed the uefi-stdio-improve branch from 3c9cae1 to d066bdf Compare December 17, 2023 14:38

Ayush1325 added 3 commits January 20, 2024 01:46

Improve UEFI stdio

07000dc

- Do not drop any character while reading - eabdf == Unsupported status - loop untill read character or error encountered Signed-off-by: Ayush Singh <ayushdevel1325@gmail.com>

Use heap for stdout and stderr

346472c

Signed-off-by: Ayush Singh <ayushdevel1325@gmail.com>

Fixes from PR

1fbb00b

- is_ebadf always returns false - Allow reading partial characters to buffer - Allow full UTF-16 in stdin Signed-off-by: Ayush Singh <ayushdevel1325@gmail.com>

Ayush1325 force-pushed the uefi-stdio-improve branch from d066bdf to 1fbb00b Compare January 19, 2024 20:45

Ayush1325 requested a review from workingjubilee January 19, 2024 20:57

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 22, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 22, 2024

bors merged commit 026b3b8 into rust-lang:master Feb 22, 2024

rustbot added this to the 1.78.0 milestone Feb 22, 2024

Ayush1325 deleted the uefi-stdio-improve branch February 22, 2024 10:11

Ayush1325 mentioned this pull request Jul 6, 2024

Tracking Issue for uefi-std #100499

Open

Uh oh!

Conversation

Ayush1325 commented Oct 25, 2023

Uh oh!

rustbot commented Oct 25, 2023

Uh oh!

Uh oh!

Uh oh!

workingjubilee commented Oct 30, 2023

Uh oh!

Uh oh!

Uh oh!

workingjubilee commented Oct 30, 2023

Uh oh!

Uh oh!

Ayush1325 commented Oct 31, 2023

Uh oh!

workingjubilee commented Oct 31, 2023

Uh oh!

dvdhrm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

workingjubilee commented Nov 22, 2023

Uh oh!

Ayush1325 commented Nov 30, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bors commented Jan 13, 2024

Uh oh!

Amanieu commented Feb 21, 2024

Uh oh!

workingjubilee commented Feb 22, 2024

Uh oh!

bors commented Feb 22, 2024

Uh oh!

bors commented Feb 22, 2024

Uh oh!

bors commented Feb 22, 2024

Uh oh!

rust-timer commented Feb 22, 2024

Overall result: ✅ improvements - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants