Improve docs and tests for zircon_object::signal

[xxchan]

Content of this PR

• zircon_object::signal passed #![deny(missing_docs)], but #![allow(missing_docs)] is added in signal::port_packet.
  Since port_packet just defines some C-style data types and the names can convey meanings, I think it is fine to omit docs.
• zircon_object::signal test coverage increases to 96.4%.
• Some minor refactors
  • validate options in Port::new()
  • refactor FutexFuture::drop: using waiter.inner.futex instead of waiter.thread.proc().get_futex()
• Some minor fixes
  • The doc of port.wait says it takes out all packets, but only takes out one in fact.
  • The return count of futex.wake is wrong.

PTAL and see whether the changes are proper. Besides, I still have

Some questions

timer

1. Slack is useless. What's the plan about it?

port

1. Is it OK that many structs in port_packet are public?
2. Where should the validation of options be done? Low level Port::new() or high level syscalls? Or both? Should Interrupt::bind() validate port.can_bind_to_interrupt() again? (It is not validated now, so we can pass a port without BIND_TO_INTERUPT option actually)
   I'm a little bit confused about such kind of questions.

futex

I'm not very familiar with futex, so I might have understood some concepts wrong

1. Should futex only be created by proc.get_futex() and not by Futex::new? Otherwise the futex cannot be bound to some thread. This question arises from this situation:

static VALUE: AtomicI32 = AtomicI32::new(1);
let futex = Futex::new(&VALUE);

let root_job = Job::root();
let proc = Process::create(&root_job, "proc", 0).expect("failed to create process");
let thread = Thread::create(&proc, "thread", 0).expect("failed to create thread");
assert!(!Arc::ptr_eq(&futex, &thread.proc().get_futex(&VALUE)));

This is kind of like the question mentioned above about validating port options, where the low level primitives are misused . So who is responsible to ensure the correctness? Should the object methods be robust enough themselves, or just let the user obey some contracts?
2. What's the use of waiter.thread (and the parameter thread in Futex::wait_with_owner())? It is reasonable that the waiter knows which thread is waiting...But it can be set to None? And I think it is not used anywhere actually.

[coveralls]

Pull Request Test Coverage Report for Build 202035729

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 196 of 202 (97.03%) changed or added relevant lines in 8 files are covered.
• 16 unchanged lines in 3 files lost coverage.
• Overall coverage increased (+3.9%) to 37.857%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
zircon-object/src/signal/port.rs	14	15	93.33%
zircon-object/src/signal/port_packet.rs	72	73	98.63%
zircon-syscall/src/port.rs	0	1	0.0%
zircon-object/src/signal/futex.rs	71	74	95.95%

Files with Coverage Reduction	New Missed Lines	%
../../../../../usr/share/rust/.cargo/registry/src/github.com-1ecc6299db9ec823/async-task-3.0.0/src/raw.rs	4	67.83%
zircon-object/src/task/thread.rs	5	49.56%
../../../../../usr/share/rust/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-channel-0.3.5/src/oneshot.rs	7	59.55%

Totals
Change from base Build 201900295:	3.9%
Covered Lines:	4871
Relevant Lines:	12867

---

💛 - Coveralls

[xxchan]

One more question: I found async-booc says here that:

Importantly, we have to update the Waker every time the future is polled because the future may have moved to a different task with a different Waker. This will happen when futures are passed around between tasks after being polled.

Is that needed in zCore?

[xxchan]

Another question: there is a EventPair::peer(), event0.peer() will call it instead of the trait function. Is this intended?

[wangrunji0408]

Looks good!

[wangrunji0408]

Sorry for my late response. I was busy on the zCore summer of code last week. QAQ

[wangrunji0408]

About the Questions

Timer

Slack is useless. What's the plan about it?

Seems it's not critical. We can consider removing this code.

Port

Is it OK that many structs in port_packet are public?

I prefer to make them private if possible.

Where should the validation of options be done? Low level Port::new() or high level syscalls? Or both?

High level syscalls, I think.

Should Interrupt::bind() validate port.can_bind_to_interrupt() again? (It is not validated now, so we can pass a port without BIND_TO_INTERUPT option actually)

Yes. So you found a bug :)

Futex

Should futex only be created by proc.get_futex() and not by Futex::new?

For syscall layer, use proc.get_futex() only. This is a kind of contract but never be mentioned before...

So who is responsible to ensure the correctness? Should the object methods be robust enough themselves, or just let the user obey some contracts?

In general, objects should be designed to be robust enough. But in the case of futex, it doesn't know the concept of memory space or process. So when using futex within multiple processes, it's the user(syscall layer)'s response to make it correct.

What's the use of waiter.thread (and the parameter thread in Futex::wait_with_owner())?

I'm not familiar with the implementation details of futex with threads. Maybe @PanQL and @benpigchu can give a help.

Waker

Importantly, we have to update the Waker every time the future is polled because the future may have moved to a different task with a different Waker.

Yes I noticed that. This rule should be followed in all leaf future implementations. Although this issue has not brought trouble so far, it should be fixed later.

EventPair

there is a EventPair::peer(), event0.peer() will call it instead of the trait function. Is this intended?

Yes. When you get an object which is known to be a EventPair, its peer should be the same type intuitively.

[xxchan]

Thank you for answering me ☺

[xxchan]

Where should the validation of options be done? Low level Port::new() or high level syscalls? Or both?

High level syscalls, I think.

Is that because we can early exit the syscall if the option is invalid?

If so, we can use from_bits() in syscall, and from_bits_truncate() in object.

[xxchan]

Oh, we can directly use typed bitflags like PortOptions in objects